Last modified: 2014-08-09 01:35:27 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T54210, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 52210 - closeElement causes UI breakage when $wgWellFormedXml = false;, does not conform to spec
closeElement causes UI breakage when $wgWellFormedXml = false;, does not conf...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Matthew Flaschen
:
Depends on:
Blocks: 50040
  Show dependency treegraph
 
Reported: 2013-07-29 06:28 UTC by Krinkle
Modified: 2014-08-09 01:35 UTC (History)
4 users (show)

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


Attachments
Screenshot of where the border is broken (238.02 KB, image/png)
2013-07-29 06:28 UTC, Krinkle
Details
Screenshot of where border isn't broken. (206.02 KB, image/png)
2013-07-29 06:28 UTC, Krinkle
Details

Description Krinkle 2013-07-29 06:28:06 UTC
Created attachment 12999 [details]
Screenshot of where the border is broken

After almost 2 hours of debugging I finally figured out what caused the tabs in Monobook to sometimes cause the border to be misaligned (see attachment).

For some reason the <a> inside the <li> was smaller than the <li>, which is odd since the size of the <li> is solely determined by its content.

More over, it displays right on mediawiki.org and beta.wmflabs.org, but broken on translatewiki.net and my local wiki.

The DOM (when inspected) looks completely the same, and no CSS styles applied differently.

Then I looked at the DOM and notice that on the "good" wikis, the output is:

<h2>Navigation menu</h2>
<div id="p-cactions" class="portlet" role="navigation">
	<h3>Views</h3>
	<div class="pBody">
		<ul>
			<li id="ca-nstab-main" class="selected"><a href="/wiki/Sandbox" ..>Page</a></li>
			<li id="ca-talk"><a href="/wiki/Talk:Sandbox" ..>Discussion</a></li>



And on the "bad" wikis, the output is:
	
<h2>Navigation menu</h2>
<div id="p-cactions" class="portlet" role="navigation">
	<h3>Views</h3>
	<div class="pBody">
		<ul>
			<li id=ca-nstab-main class=selected><a href="/wiki/Main_Page" ..>Page</a>
			<li id=ca-talk class=new><a href="/w/index.php?title=Talk:Main_Page&amp;action=edit&amp;redlink=1" ..>Discussion</a>


After ruling out Tidy, I narrowed it down to $wgWellFormedXml.

Or more precisely by Html::closeElement (called from SkinTemplate::makeListItem, called from SkinMonobook::cactions()), where it omits the closing tag of an <li>.

As a result, the browser only closes the <li> when it encounters the next opening tag, and as such includes a text node between </a> and (the implied) </li>.

This extra space causes the list item to be wider than the <a> it holds.
Comment 1 Krinkle 2013-07-29 06:28:29 UTC
Created attachment 13000 [details]
Screenshot of where border isn't broken.
Comment 2 Krinkle 2013-07-29 06:29:44 UTC
Reproduced the bug in Chrome 28 (stable), Firefox 22 (stable) and Chrome 30 (canary), Mac.
Comment 3 Matthew Flaschen 2014-08-07 14:58:07 UTC
It also does not conform to the Living Standard (the same source it cites) (http://www.whatwg.org/html/syntax.html#optional-tags).  There are all sorts of rules about when you can omit the tag.  E.g.

"A head element's end tag may be omitted if the head element is not immediately followed by a space character or a comment."

"An li element's end tag may be omitted if the li element is immediately followed by another li element or if there is no more content in the parent element."

Since closeElement does not take into account context, it does not follow these rules.  In some cases, the DOM might be invalid anyway if you don't follow these rules (haven't looked into it), but in other's, it's clearly valid.  For example, it's perfectly fine to have a comment immediately after the head element; but closeElement can't know that, so it violates the spec.

I'm going to put up a patch to just remove this whole thing.
Comment 4 Gerrit Notification Bot 2014-08-07 16:28:43 UTC
Change 152778 had a related patch set uploaded by Mattflaschen:
Html::closeElement: Don't omit closing tags.

https://gerrit.wikimedia.org/r/152778
Comment 5 Gerrit Notification Bot 2014-08-08 00:43:35 UTC
Change 152778 merged by jenkins-bot:
Html::closeElement: Don't omit closing tags.

https://gerrit.wikimedia.org/r/152778

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


Navigation
Links