Page MenuHomePhabricator

MobileFrontend extension outputting invalid HTML in footer
Closed, ResolvedPublic

Description

When visiting a page such as http://en.wikipedia.org/wiki/Main_Page?useformat=mobile currently, the code at the bottom of the page looks like this:

<div id='copyright'>Text is available under the <a rel="license" href="http://en.wikipedia.org/wiki/Wikipedia:Text_of_Creative_Commons_Attribution-ShareAlike_3.0_Unported_License">Creative Commons Attribution-ShareAlike License</a><a rel="license" href="http://creativecommons.org/licenses/by-sa/3.0/" style="display:none;"></a>;
additional terms may apply.
See <a href="http://wikimediafoundation.org/wiki/Terms_of_use">Terms of use</a> for details.<br/>
Wikipedia&reg; is a registered trademark of the <a href="http://www.wikimediafoundation.org/">Wikimedia Foundation, Inc.</a>, a non-profit organization.<br /></li><li class="noprint"><a class='internal' href="http://en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a></div>

There are a number of problems with this code. It's using some bizarre display:none inline CSS, it specifies a <li> element without any <ol> or </ul> pair, and one of the <li> elements is left unclosed.

I'm told that this isn't exactly MobileFrontend's fault, but regardless, MobileFrontend is the extension responsible for outputting sane and clean code to mobile devices.


Version: unspecified
Severity: normal

Details

Reference
bz30406

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 11:54 PM
bzimport set Reference to bz30406.

preilly wrote:

This content comes from the following call:

$skin = $wgUser->getSkin();
$copyright = $skin->getCopyright();

So, it's not directly from the Mobile Frontend extension.

I only get

<div id="copyright">Text is available under the <a href="http://creativecommons.org/licenses/by-sa/3.0/">Creative Commons Attribution/Share-Alike License</a>;
additional terms may apply.
See <a href="http://wikimediafoundation.org/wiki/Terms_of_Use">Terms of Use</a> for details.</div>

http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/Skin.php?view=annotate#l721

preilly wrote:

I see the following on en.wikipedia.org:

<div id="copyright">Text is available under the <a rel="license" href="http://en.wikipedia.org/wiki/Wikipedia:Text_of_Creative_Commons_Attribution-ShareAlike_3.0_Unported_License">Creative Commons Attribution-ShareAlike License</a><a rel="license" href="http://creativecommons.org/licenses/by-sa/3.0/" style="display:none;"></a>;
additional terms may apply.
See <a href="http://wikimediafoundation.org/wiki/Terms_of_use">Terms of use</a> for details.<br>
Wikipedia® is a registered trademark of the <a href="http://www.wikimediafoundation.org/">Wikimedia Foundation, Inc.</a>, a non-profit organization.<br><li class="noprint"><a class="internal" href="http://en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a></li></div>

Ah, ok.

So that's logged out, when you're logged in, you get what I pasted

So why do I get different?

I'm using Vector, which is the site default skin, no custom JS or CSS, but I get a different copyright...

preilly wrote:

I've got no idea. I'm trying to figure that out right now.

This looks like a bug with Wikipedia config, nothing to do with core or extensions.
https://secure.wikimedia.org/wikipedia/en/w/index.php?title=MediaWiki:Wikimedia-copyright&action=edit

It also appears to be a raw html outputting message, so I can't even argue that invalid <li>'s like that should be wrapped automatically in a <ul> by the parser.

Assigning to James Alexander who'll coordinate with the EN Admins about this change.

(In reply to comment #9)

Assigning to James Alexander who'll coordinate with the EN Admins about this
change.

I talked to James tonight and poked at this bug briefly. Basically the English Wikipedia is using a hack to insert an extra list item element (<li>) into the footer, because they want a "Contact us" link down there, but there's no native support for adding a list item element to <ul id="f-list">. It looks like completely broken code, but in context, it makes much more sense:

From http://en.wikipedia.org/wiki/Main_Page:


<ul id="f-list">

		<li id="lastmod"> This page was last modified on 26 July 2011 at 09:03.<br /></li> 
		<li id="copyright">Text is available under the <a rel="license" href="http://en.wikipedia.org/wiki/Wikipedia:Text_of_Creative_Commons_Attribution-ShareAlike_3.0_Unported_License">Creative Commons Attribution-ShareAlike License</a><a rel="license" href="http://creativecommons.org/licenses/by-sa/3.0/" style="display:none;"></a>;

additional terms may apply.
See <a href="http://wikimediafoundation.org/wiki/Terms_of_use">Terms of use</a> for details.<br/>
Wikipedia&reg; is a registered trademark of the <a href="http://www.wikimediafoundation.org/">Wikimedia Foundation, Inc.</a>, a non-profit organization.<br /></li><li class="noprint"><a class='internal' href="http://en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a></li>

		<li id="privacy"><a href="http://wikimediafoundation.org/wiki/Privacy_policy" title="wikimedia:Privacy policy">Privacy policy</a></li> 
		<li id="about"><a href="/wiki/Wikipedia:About" title="Wikipedia:About">About Wikipedia</a></li> 
		<li id="disclaimer"><a href="/wiki/Wikipedia:General_disclaimer" title="Wikipedia:General disclaimer">Disclaimers</a></li> 
		<li id="mobileview"><a href='/w/index.php?title=Main_Page&amp;useformat=mobile'>Mobile view</a></li>

</ul>

In context, it gets wrapped properly with <ul>, it ends the copyright's <li>, and then inserts a "Contact us" <li>.

I don't know if a bug already exists, but there should be native support for adding a list item to the footer list items. Then this horrible hack can be killed properly. MobileFrontend doesn't really need to be changed here (and shouldn't be!) and isn't really to blame, this is simply a lack of functionality in MediaWiki core that the English Wikipedia is trying to work around (by abusing raw HTML...).

(In reply to comment #2)

I only get

<div id="copyright">Text is available under the <a
href="http://creativecommons.org/licenses/by-sa/3.0/">Creative Commons
Attribution/Share-Alike License</a>;
additional terms may apply.
See <a href="http://wikimediafoundation.org/wiki/Terms_of_Use">Terms of Use</a>
for details.</div>

http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/Skin.php?view=annotate#l721

Logged in while visiting what URL? In what browser? And have you opted in to anything mobile-related?

I should mention that it is possible to change the footerlinks list:
Use the SkinTemplateOutputPageBeforeExec to modify either the info or places array of the footerlinks $tpl key adding a new tpl key to it. And $tpl->set the key you added.

So as I understand it the problem lies outside MobileFrontend.
The issue is that the copyright text is printed inside a li element (#footer-info-copyright)
The page does a html hack to create another list item containing a contact link (http://en.wikipedia.org/wiki/MediaWiki:Wikimedia-copyright)

This is becoming less important with the footer redesign and also I see this as an issue that Wikipedia admins must address (I've said so on the talk page - http://en.wikipedia.org/wiki/MediaWiki_talk:Wikimedia-copyright#Footer_invalid_html)

(In reply to comment #13)

So as I understand it the problem lies outside MobileFrontend.
The issue is that the copyright text is printed inside a li element
(#footer-info-copyright)
The page does a html hack to create another list item containing a contact link
(http://en.wikipedia.org/wiki/MediaWiki:Wikimedia-copyright)

This is becoming less important with the footer redesign and also I see this as
an issue that Wikipedia admins must address (I've said so on the talk page -
http://en.wikipedia.org/wiki/MediaWiki_talk:Wikimedia-copyright#Footer_invalid_html)

Wikipedia admins cannot fix this issue. They don't have the ability to add new list items. Someone needs to make a configuration change to Wikimedia's setup to allow them to have a separate list item for that.

(In reply to comment #13)

So as I understand it the problem lies outside MobileFrontend.
The issue is that the copyright text is printed inside a li element
(#footer-info-copyright)
The page does a html hack to create another list item containing a contact link
(http://en.wikipedia.org/wiki/MediaWiki:Wikimedia-copyright)

This is becoming less important with the footer redesign and also I see this as
an issue that Wikipedia admins must address (I've said so on the talk page -
http://en.wikipedia.org/wiki/MediaWiki_talk:Wikimedia-copyright#Footer_invalid_html)

Re-opening this for now. At the http://en.m.wikipedia.org, we now see the following code:


			  <div id='copyright'><ul id="footer-info"><li>Text is available under the <a rel="license" href="//en.wikipedia.org/wiki/Wikipedia:Text_of_Creative_Commons_Attribution-ShareAlike_3.0_Unported_License">Creative Commons Attribution-ShareAlike License</a><a rel="license" href="//creativecommons.org/licenses/by-sa/3.0/" style="display:none;"></a>;

additional terms may apply.
See <a href="wikimediafoundation.org/wiki/Terms_of_use">Terms of use</a> for details.<br/>
Wikipedia&reg; is a registered trademark of the <a href="
www.wikimediafoundation.org/">Wikimedia Foundation, Inc.</a>, a non-profit organization.<br /></li><li class="noprint"><a class='internal' href="//en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a></li></ul></div>

			</div>

This looks better. There's now a closing </li> and a closing </ul> However, https://en.wikipedia.org/wiki/MediaWiki:Wikimedia-copyright hasn't been changed, so the question becomes: where did these tags come from?

I haven't had a chance to investigate this. I have suspicions about the current behavior, but I'm re-opening the bug for now.

(In reply to comment #15)

This looks better. There's now a closing </li> and a closing </ul> However,
https://en.wikipedia.org/wiki/MediaWiki:Wikimedia-copyright hasn't been
changed, so the question becomes: where did these tags come from?

I haven't had a chance to investigate this. I have suspicions about the current
behavior, but I'm re-opening the bug for now.

Uhh, right. So there's this lovely bit of code from r100057, r100059, and r102441:


		$copyright = $skin->getCopyright();
		if ( stristr( $copyright, '<li class="noprint">' ) !== false ) {
			$copyright = '<ul id="footer-info"><li>' . $copyright . '</li></ul>';
		}

This is a hack (and an undocumented one at that). This should be properly addressed in the MobileFrontend extension.

Why cannot Wikipedia Admins fix this? I don't know why on this page it needs to use a new list item when instead it could use other html markup in this instance.

My suggestion in the talk page was:

Please could we change </li><li class="noprint"><a class='internal' href="en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a>
We cannot assume that the copyright will always be printed in a list element - in the mobile site for instance it isn't and leads to invalid html (see https://bugzilla.wikimedia.org/show_bug.cgi?id=30406)
I believe this has the following line has the same stylistic effect without leading to bad HTML elsewhere: <p class="noprint"><a class="internal" href="
en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a></p>

Is this not possible?

We certainly don't want the undocumented hack above.

(In reply to comment #17)

Why cannot Wikipedia Admins fix this? I don't know why on this page it needs to
use a new list item when instead it could use other html markup in this
instance.

My suggestion in the talk page was:

Please could we change </li><li class="noprint"><a class='internal'
href="en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a>
We cannot assume that the copyright will always be printed in a list element -
in the mobile site for instance it isn't and leads to invalid html (see
https://bugzilla.wikimedia.org/show_bug.cgi?id=30406)
I believe this has the following line has the same stylistic effect without
leading to bad HTML elsewhere: <p class="noprint"><a class="internal"
href="
en.wikipedia.org/wiki/Wikipedia:Contact_us">Contact us</a></p>

Is this not possible?

Look at the page in Monobook, silly. (And re-read some of the previous comments on this bug regarding why it's using an <li>. It's fully explained.)

We certainly don't want the undocumented hack above.

Sure, so remove it. That has nothing to do with English Wikipedia administrators, though.

Okay thanks MZMcBride - this makes a *lot* more sense when looking at Wikipedia with the monobook theme thanks for clearing that up for me. Much appreciated.

I believe I have a better fix for this - see https://gerrit.wikimedia.org/r/4122 which feels like a much cleaner solution to the problem and is documented and at least cleans up the problem MFE side (imo).

I would suggest that Wikipedia.org should add a footer link to its configuration via a method outside the MediaWiki:Wikimedia-copyright page - am not sure if this can be done already - Daniel Friesen suggests that it might be but if not could someone who understands this more than me open a new bug?

A bit strange to use an HTML comment instead of a PHP comment (the former now displays in the HTML source), but other than that, this seems to be acceptable. Thanks for your work on this!