Page MenuHomePhabricator

Do not output empty portlets
Closed, DuplicatePublic

Description

Do not output empty portlets.

  1. It is not valid. <ul></ul> is invalid.
  1. They are not shown, but they are present in HTML, so: a) useless cluttering of output code b) unnecesssary increase of DOM structure c) accessibility issues

Version: unspecified
Severity: minor

Details

Reference
bz24500

Related Objects

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:03 PM
bzimport set Reference to bz24500.
Danny_B created this task.Jul 22 2010, 7:34 PM

Fairly sure this was addressed in bug 23015 and bug 23575.

(In reply to comment #1)

Fairly sure this was addressed in bug 23015 and bug 23575.

Yes, this is a duplicate. Also, there's, what, one or two potentially empty <ul>s on the entire page?

  • This bug has been marked as a duplicate of bug 23015 ***

Please read carefuly. It is not only about empty <ul>.

The entire empty portlet should not be there at all.

Why should e.g. be there "Variant" portlet in languages, which do not use variants? Etc.

Reopening, since this is not about the validity only. (Besides, untill we will serve HTML5 (and that might take long time, it won't be in few months), it still IS invalid.)

ayg wrote:

(In reply to comment #3)

Reopening, since this is not about the validity only. (Besides, untill we will
serve HTML5 (and that might take long time, it won't be in few months)

It could be a few months, or less.

(In reply to comment #3)

Please read carefuly. It is not only about empty <ul>.
The entire empty portlet should not be there at all.
Why should e.g. be there "Variant" portlet in languages, which do not use
variants? Etc.

This specific argument make sense: the variant portlet should be safe to drop for languages without variants. Repurposing bug.

Reopening, since this is not about the validity only. (Besides, untill we will
serve HTML5 (and that might take long time, it won't be in few months), it
still IS invalid.)

As outlined on one of the previous bug, we care more about not breaking scripts than we care about such a harmless invalidity: the rule that <ul>s can't be empty is a stupid one that all browsers I know of ignore (which makes perfect sense) ands which was dropped in HTML5, probably for these reasons (most of the premise of the simplified HTML5 markup is that it exploits tolerances in existing browsers' HTML parsers, AFAIK)

(In reply to comment #5)

(In reply to comment #3)

Please read carefuly. It is not only about empty <ul>.
The entire empty portlet should not be there at all.
Why should e.g. be there "Variant" portlet in languages, which do not use
variants? Etc.

This specific argument make sense: the variant portlet should be safe to drop
for languages without variants. Repurposing bug.

Special pages have also empty "Actions" and "Views".

Why should user using agent without CSS see things he doesn't need to see at all? Didn't we want to simplify things? Why do we out of sudden return useless things we never used to return?

Reopening, since this is not about the validity only. (Besides, untill we will
serve HTML5 (and that might take long time, it won't be in few months), it
still IS invalid.)

As outlined on one of the previous bug, we care more about not breaking scripts
than we care about such a harmless invalidity: the rule that <ul>s can't be
empty is a stupid one that all browsers I know of ignore (which makes perfect
sense) ands which was dropped in HTML5, probably for these reasons (most of the
premise of the simplified HTML5 markup is that it exploits tolerances in
existing browsers' HTML parsers, AFAIK)

We should care about the validity first. If something isn't valid, it makes more difficult to convert it or its displaying might be rejected. Don't think only about "all browsers you know" - there are not only browsers, there are also random other tools, which rely or depend on validity.

Besides, MediaWiki has variable to decide if you want to output HTML5 or XHTML1. That means, even if we will switch to HTML5 by default somewhen in far future, that we still have to take care about XHTML1.

We should also care about optimalization - why should user load unnecessary and useless data, which are hidden and won't be used? Not everybody is sitting in USA on optical cable, you know? Lot of people still pay a lot per kB, not small monthly flat fees.

The client scripts are the last piece in chain of website design, not the first. We can not build the house from the roof. Fix the scripts then, if they get broken by missing portlet. That is the proper solution. Not messing the HTML. The scripts are supposed to accomodate to HTML not HTML to scripts.

(In reply to comment #6)

Special pages have also empty "Actions" and "Views".
Why should user using agent without CSS see things he doesn't need to see at
all? Didn't we want to simplify things? Why do we out of sudden return useless
things we never used to return?

Yes, but there are scripts manipulating these portlets as explained previously.

We should care about the validity first. If something isn't valid, it makes
more difficult to convert it or its displaying might be rejected. Don't think
only about "all browsers you know" - there are not only browsers, there are
also random other tools, which rely or depend on validity.

I have a very very hard time imagining how a browser could possibly choke on an empty <ul> in any way, shape or form. It makes zero sense for a browser implementor to specifically enforce this requirement, because being tolerant is easier, and programmers are lazy.

In general, yes, I agree that validity is important, but I believe we can make exceptions for such utterly brain-dead things as requiring (not saying "you shouldn't do this" but saying "you MUST NOT do this, validation will FAIL if you do this") that <ul>s not be empty. HTML5 has realized this and has removed this requirement.

We should also care about optimalization - why should user load unnecessary and
useless data, which are hidden and won't be used? Not everybody is sitting in
USA on optical cable, you know? Lot of people still pay a lot per kB, not small
monthly flat fees.

One empty portlet takes 158 bytes (before gzip). By contrast, the HTML of the enwiki main page is 57,629 bytes (before gzip, in Vector; Monobook is 406 bytes shorter). There are bigger fish to fry here, such as the many <link rel="stylesheet" href="veryLongURLHere"> tags, <script src="foo"> tags and the <script> tag with the huge pile of JS vars (the resource loader will address at least the first two).

The client scripts are the last piece in chain of website design, not the
first. We can not build the house from the roof. Fix the scripts then, if they
get broken by missing portlet. That is the proper solution. Not messing the
HTML. The scripts are supposed to accomodate to HTML not HTML to scripts.

"Just fix the scripts" isn't that easy where e.g. user and site scripts are involved. IMO this is too much hassle for rather minimal gain, as most pages don't have empty namespaces or cactions portlets anyway. I do agree with removing the variants portlet if the content language has no variant support, because in that case it's quite clear you will never need the variants portlet anywhere on the entire wiki.

  • Bug 25210 has been marked as a duplicate of this bug. ***
demon added a comment.Sep 29 2010, 2:09 PM
  • Bug 25366 has been marked as a duplicate of this bug. ***
  • Bug 25907 has been marked as a duplicate of this bug. ***

giecrilj wrote:

In general, yes, I agree that validity is important, but I believe we can make
exceptions for such utterly brain-dead things as requiring (not saying "you
shouldn't do this" but saying "you MUST NOT do this, validation will FAIL if
you do this") that <ul>s not be empty. HTML5 has realized this and has removed
this requirement.

If you prefer HTML5, that is your choice. However, the document does not meet the requirements of XHTML 1.0 and this declaration should be removed, or changed to

"-//WikiMedia//DTD XHTML 1.0 Improved//EN"

giecrilj wrote:

(In reply to comment #7)

I have a very very hard time imagining how a browser could possibly choke on an
empty <ul> in any way, shape or form. It makes zero sense for a browser
implementor to specifically enforce this requirement, because being tolerant is
easier, and programmers are lazy.

OTOH, validating XML processors, such as Microsoft XML Core Services, bale out on such irregularities by default.

(In reply to comment #11)

If you prefer HTML5, that is your choice. However, the document does not meet
the requirements of XHTML 1.0 and this declaration should be removed, or
changed to

"-//WikiMedia//DTD XHTML 1.0 Improved//EN"

We want to change the doctype to HTML5, and we will (there's support for this in MediaWiki already through $wgHTML5 = true or something), but there are issues to be sorted out (entities breaking XML parsers and such) and someone needs to have time to do it.

ayg wrote:

(In reply to comment #11)

If you prefer HTML5, that is your choice. However, the document does not meet
the requirements of XHTML 1.0 and this declaration should be removed, or
changed to

"-//WikiMedia//DTD XHTML 1.0 Improved//EN"

It will be changed to <!DOCTYPE html> in due course. We haven't gotten around to it yet because no sysadmins seem interested in overseeing the transition, which will have some side effects that need to be dealt with. There's no point in us going to much trouble to obey XHTML 1.0 rules when they'll be moot as soon as we switch. The MediaWiki default has already been to use <!DOCTYPE html> for close to two years now, IIRC -- Wikipedia just hasn't switched yet. XHTML 1.0 is obsolete and should be ignored.

(In reply to comment #12)

OTOH, validating XML processors, such as Microsoft XML Core Services, bale out
on such irregularities by default.

Fortunately, our websites are designed for use by web browsers, not validating XML processors. Anyone who wants to screen-scrape the site for some reason gets to deal with any fallout themselves. For instance, they could use the API, or configure their XML processor to be non-validating, or use an HTML parser instead of an XML parser.

If anyone is having serious difficulties because of this bug in real life, they can step forward and say so. Purely hypothetical arguments carry no weight.

  • Bug 26035 has been marked as a duplicate of this bug. ***
  • This bug has been marked as a duplicate of bug 23026 ***

ayg wrote:

Making this bug the main bug, because it has more discussion than bug 23026.

ayg wrote:

*** Bug 23026 has been marked as a duplicate of this bug. ***

Just nothing here for the record, as being reported in "MediaWiki core > Vector skin" this is no longer valid as MediaWiki has wgHTML5 enabled by default on all new wikis.

Wikimedia's own installations still have it disabled but that's an issue unrelated to the software, which is already on the agenda to be enabled.

Reopening. I don't know why was this closed; the issue is not fixed. The discussion above seems to be mostly bikeshedding about XHTML/HTML5, which is completely unrelated.

This causes the inclusion of completely unnecessary HTML (example pasted below). If it's not needed for anything, why is it there?

If some JS user-scripts blow up when menus are missing, then

  • it's trivial to fix ("if(!menu) return;")
  • that does not matter anyway (I suppose all those scripts do is adding new tabs, and if there's nowhere to add them, not much else than not adding the tab could be done - and that's what erroring out does anyway)

Example:

<!-- 1 -->
<div id="p-variants" class="vectorMenu emptyPortlet">
<h4>

		</h4>

<h5><span>Variants</span><a href="#"></a></h5>
<div class="menu">

		<ul>
					</ul>

</div>
</div>

<!-- /1 -->

Making userscripts not output tabs that they are programmed to output is NOT a fix.
And there IS a place for these userscripts to add their tabs. It's just empty. Whether it has elements on-load or not is irrelevant. It's an area inside of the skin so it's perfectly valid for a userscript to a append a link and then unhide the portlet (remove the emptyPortlet style).

Then explain why #p-lang (LANGUAGES portal) is not outputted on Vector if it would have been empty. (I actually had to work around this in LivePreview.)

Maybe something like :empty should be used in CSS instead of .emptyPortlet?

Simple inconsistent programming. If you need to add some languages dynamically feel free to open a bug to have #p-lang outputted as an .emptyPortlet.

:empty can't be used. Compatibility wise it doesn't even work in IE8. But besides that it wouldn't work anyways. We can't hide the entire .vectorMenu with :empty because it still has stuff inside of it.

Basically it has a use for scripted content. It doesn't actually break anything. And complaints about the few bytes that it adds to the markup are pointless. So there's no reason to bother removing it.

Exactly.

  • The HTML-validness of empty portlets is an invalid problem per bug 23015 and bug 23575.
  • Empty portlets in general is because different skins output portlets in different positions, by outputting empty placeholders, dynamic insertion of portlets becomes possible (primarily through mw.util.addPortletLink.
  • There not being empty portlets for others is probably because nobody has added them yet.

Stupid question, maybe, but I've wondered this for a long time and am not aware of any relevant history/discussion: why not just have addPortletLink check if the specified portlet exists in the DOM and, if not, generate it? That allows this bug to be fixed with no adverse side effects (in theory; if a userscript is doing something retarded such as generating portlet links "manually" and this would break it, it probably deserves what it gets) as well as providing a built-in interface for creating new portlets. This does create the potential problem of where the new portlet would get added, but that's nothing a separate function and a sensible default couldn't handle.

(In reply to comment #26)

Why not just have addPortletLink check if the specified portlet exists in the DOM and
, if not, generate it?

Its not a stupid question, but I did answer it already. The reason the placeholders are needed is because skins generate their portlets all in different ways (and even those that don't may do so in the future). We have at least 3 or 4 different html structures so far.

And aside from knowing the HTML template for the portlet, the position in the dom is just as variable (the order of portlets as well as the placement in the larger structure). For example, some skins don't have a sidebar.

Its not a stupid question, but I did answer it already. The reason the
placeholders are needed is because skins generate their portlets all in
different ways (and even those that don't may do so in the future). We have at
least 3 or 4 different html structures so far.
And aside from knowing the HTML template for the portlet, the position in the
dom is just as variable (the order of portlets as well as the placement in the
larger structure). For example, some skins don't have a sidebar.

The portlets use the same ID between skins, though, right? In that case, we could have a separate function for each HTML template used to generate the portlets, and a main function that chooses the right generator and inserts into the proper DOM location according to the skin... though I'm not sure how that would work in light of custom skins (the only thing I can think of is a wrapper that calls a skin-specific function, which then calls one of the generators and inserts the resulting object into the DOM, but that has problems of its own...). I guess it'd be more trouble than it's worth, then.

Note that debian is patching this to generate valid html (I haven't tested if scripts break, do we even have a list of affected ones?). It would be nice to fix it upstream.

(In reply to comment #29)

Note that debian is patching this to generate valid html (I haven't tested if
scripts break, do we even have a list of affected ones?). It would be nice to
fix it upstream.

Got a reference to the changeset they did that in?

You talking about <ul></ul> without a <li> being invalid xhtml? Frankly it's irrelevant since we're aiming for HTML5, not XHTML. And <ul></ul> without a <li> IS valid HTML5. We have no reason to listen to XHTML on this one because it was just one of those worthless restrictions that served no purpose, no one-implemented, and now the standard have been updated to match reality. Debian /fixing/ this is just an artifact of them not updating past 1.15 for years and hence still using XHTML up till very very recently.

Got a reference to the changeset they did that in?

http://anonscm.debian.org/viewvc/pkg-mediawiki/mediawiki/trunk/debian/patches/fix_invalid_xhtml.patch

There are several xhtml issues patched there.

You talking about <ul></ul> without a <li> being invalid xhtml?

Yes. Invalid XHTML / HTML4.

Frankly it's irrelevant since we're aiming for HTML5, not XHTML.

I know. But we can run with $wgHtml5 = false;

Debian /fixing/ this is just an artifact of them not updating past 1.15 for
years and hence still using XHTML up till very very recently.

And FusionForge showing XHTML errors like big bad errors. Feel free to try to convince Thorsten on this.

I wonder, would doing

echo $wgHTML5 ? "<ul></ul>" : '<ul><li style="display: none"><!--XHTML doesn't allow empty uls--></li></ul>";

be an acceptable solution for all parties?

(In reply to comment #31)

Got a reference to the changeset they did that in?
Frankly it's irrelevant since we're aiming for HTML5, not XHTML.

I know. But we can run with $wgHtml5 = false;

$wgHtml5 exists because we used to use XHTML and we needed a way to safely transition from our old tag soup to HTML5. I would say that the existence of $wgHtml5 = false; as a possible config option is an assertion of "Temporarily output the same legacy markup you used to until I can deal with the few remaining html5 issues." rather than "Output supported XHTML".

I would hazard to say that we do not actually support XHTML at all and our intent in the long run is to eliminate XHTML completely from the codebase.
Finally setting $wgHtml5 = true; on Wikipedia is a big step towards the extermination of XHTML.

;) so I'd like to just say to Debian "MediaWiki does not support XHTML."

(In reply to comment #32)

I wonder, would doing
echo $wgHTML5 ? "<ul></ul>" : '<ul><li style="display: none"><!--XHTML doesn't
allow empty uls--></li></ul>";
be an acceptable solution for all parties?

No.

(In reply to comment #33)

;) so I'd like to just say to Debian "MediaWiki does not support XHTML."

Indeed, though don't confuse it with supporting XML-validness. We support outputting valid parseable XML (e.g. where it assures proper nesting and quick-closing with <br/> instead of <br> etc.).

If a party has a problem with an empty <ul> when HTML5 is disabled, I suggest they do indeed learn an important lesson about how the internet works: Program for browsers that exist, not for a theoretical idealism called "W3C Validator". There is no point in outputting completely useless code that is not needed for any purpose other than the satisfaction of a validator, especially considering browsers don't validate either. And as long as browsers decide to differ from the specification, one has all the right to ignore the validator, especially considering that the "W3C Validator" is not a browser (and even if it would be, we probably wouldn't support it). And of course lets not forget that this is not a browser bug, it is more a flaw in the old HTML spec, that browsers corrected and has in the mean time been taken into account in the HTML5 spec. This is in the past, however cares should look for more useful things to spend time on.

  • Bug 46939 has been marked as a duplicate of this bug. ***