Last modified: 2014-09-23 23:53:15 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 16175 - Clean up the rendering of messages displayed at the top of the edit window
Clean up the rendering of messages displayed at the top of the edit window
Status: NEW
Product: MediaWiki
Classification: Unclassified
Page editing (Other open bugs)
unspecified
All All
: Low minor (vote)
: ---
Assigned To: Amir E. Aharoni
: i18n, patch, patch-need-review
Depends on: 18880
Blocks: 18521 16783
  Show dependency treegraph
 
Reported: 2008-10-29 23:44 UTC by Happy-melon
Modified: 2014-09-23 23:53 UTC (History)
6 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Updated patch, against r53416 (6.28 KB, patch)
2009-07-17 23:01 UTC, Happy-melon
Details
Better patch, still against r53416 (6.29 KB, patch)
2009-07-18 17:36 UTC, Happy-melon
Details

Description Happy-melon 2008-10-29 23:44:19 UTC
Created attachment 5485 [details]
Improve system message handling on edit page

Patch to do a couple of things to the messages that can be displayed at the top of the edit window when editing, previewing, etc.  

1) Make the log extracts displayed underneath [[MediaWiki:Protectedpagewarning]], etc, parameters to the message itself, so that styles and structures can be easily applied to the whole object.  I only made the necessary changes to the default messages to the english ones... is that bad? :S

2) Construct the log extract for [[MediaWiki:Recreate-deleted-warn]] in the same way as for the other extracts (seems more elegant, previous version was very similar to LogEventsList::showLogExtract() anyway)

3) Wrap at least most messages in an id'd div for ease of skinning.  This seems a very common operation: worth a wrapper function in OutputPage.php?

This is my first serious patch, and more to the point I don't have a working version of MediaWiki that I can test changes on, so **THIS NEEDS TESTING** before making it live anywhere... god knows what I've broken this time :D
Comment 1 Happy-melon 2009-07-17 23:01:46 UTC
Created attachment 6354 [details]
Updated patch, against r53416

Original patch is now 11,000 revisions out of date. Updated patch no longer does point 1: we have found an elegant way to work around this issue over at enwiki, so it's no longer worth me pushing a breaking change. Tested against r53416.  Overhauled EditPage::showLogs() requires the updated LogEventsList::showLogExtract() patched in bug17979
Comment 2 Happy-melon 2009-07-18 17:36:17 UTC
Created attachment 6360 [details]
Better patch, still against r53416

Version that actually *uses* the new functionality in LogEventsList::showLogExtract().  That should have been bug17979, which is actually duped to bug18880.
Comment 3 Niklas Laxström 2009-07-19 08:25:18 UTC
Why are you turning addWikiMsg and wrapWikiMsg calls to something horrible?
Comment 4 Happy-melon 2009-07-19 09:10:20 UTC
Would you rather add *another* wfMsgXxx function, that automatically adds the "mw-<message_name>" id?  I'd be equally happy with that, but I was under the impression that we were moving *away* from using zillions of little variations of wfMsg functions?  Xml::tags should be used (either directly or through such a wrapper) as good coding practice; plus it'll be useful/necessary for, eg, the transition to HTML5 that's currently being discussed on wikitech-l.  It looks like eventually we'll switch, and then we can remove the closing </div> tag and some of the attribute quotes.  I'd rather be able to do that just by changing Xml::tags than have to go trawling through the codebase looking for hardcoded divs.  This is elementary software encapsulation, no?
Comment 5 Niklas Laxström 2009-07-19 16:10:29 UTC
You can very well add the id with wrapWikiMsg too, (are you sure classes are not enough? we've got jQuery soon). Don't over engineer, because I have nowhere seen even a suggestion that we would change what Xml-functions would do. The only proposal was to create a new HTML class by Simetrical, which sounds much more sane.

What you are suggesting now, is not readable, even ignoring the unsusual indendation. And oh, did you mention that you changed some class names too? And if you really really want it, you can even combine wrapWikiMsg and Xml-functions, something like this:

->wrapWikiMsg( Xml::element( 'div', array( 'class' => 'foo', 'id' => 'daa' ), '$1' ), 'baz' );
Comment 6 Happy-melon 2009-07-19 17:12:56 UTC
(In reply to comment #5)
> You can very well add the id with wrapWikiMsg too, (are you sure classes are
> not enough? we've got jQuery soon). Don't over engineer, 
IMO this is just standard encapsulation; in the same way that we are now hunting down and destroying all hardcoded SQL statements to allow proper database abstraction, the codebase would, in an ideal world, not contain any hardcoded HTML for the same reasons.  

> because I have nowhere
> seen even a suggestion that we would change what Xml-functions would do. The
> only proposal was to create a new HTML class by Simetrical, which sounds much
> more sane.
Which would then be deployed by a steady conversion of "Xml::..." to "Html::..." across the codebase.  Which can be greped for, unlike hardcoded instances.

> 
> What you are suggesting now, is not readable, even ignoring the unsusual
> indendation. And oh, did you mention that you changed some class names too? 
Where? "error" is added to cascadeprotectedwarning and titleprotectedwarning to be consistent with longpage-error, but I don't see any classes *changed*.  I'm not wedded to the "error" class; I can see the distinction between longpage-error and the other notices.  I used "id=mw-*edit*-..." to match the one warning (longpage-error) that *does* currently have an ID; I don't like it (they should be the standard "mw-<message_name>" like everywhere else), but it was to *avoid* changing ids.


> if you really really want it, you can even combine wrapWikiMsg and
> Xml-functions, something like this:
> 
> ->wrapWikiMsg( Xml::element( 'div', array( 'class' => 'foo', 'id' => 'daa' ),
> '$1' ), 'baz' );
> 
What's better about that?  The $1 string injection in wrapWikiMsg() is *even more* obfuscated than addHTML().

As I said, I'd be perfecly happy to add another global function, but I was under the impression that we wanted fewer message functions (and global functions generally), not more.  Maybe a method in OutputPage would be preferable... $wgOut->outputMsgDiv() or somesuch...?
Comment 7 Dan Jacobson 2009-07-20 22:47:07 UTC
This bug reminds me about Bug 19848.
Comment 8 Siebrand Mazeland 2011-09-14 13:23:32 UTC
Volunteered by Amir during bug triage.
Comment 9 Sumana Harihareswara 2012-05-22 13:52:52 UTC
Amir, do you think you will have time to review and resolve this sometime soon, perhaps in your 20% time in the next few weeks?  Thanks.
Comment 10 Amir E. Aharoni 2012-05-22 13:54:12 UTC
Yep, I actually planned to go over the bugs assigned to me Real Soon Now.
Comment 11 Sumana Harihareswara 2013-02-20 17:18:55 UTC
MatmaRex, can you take a look at this?
Comment 12 Bartosz Dziewoński 2013-02-20 19:15:11 UTC
I could try to once I get some of the 17 pending patchsets I have on gerrit merged.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links