Page MenuHomePhabricator

Treeview of categories breaks in IE9
Closed, ResolvedPublic

Description

Author: heylarson

Description:
Screenshot of treeview break in IE9 browser

So far only noticed in IE9, the treeview will display a few categories as collapsible nodes, but the remainder will appear as just list items. Attached is an example of the issue. This is not an issue with IE7 or IE8, Chrome, or Firefox.


Version: unspecified
Severity: normal

Attached:

Capture.PNG (440×304 px, 9 KB)

Details

Reference
bz30557
ReferenceSource BranchDest BranchAuthorTitle
repos/releng/scap!50review/dancy/T325576-sync-world-fullmasterdancyAdd full_image_build config option
Customize query in GitLab

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:53 PM
bzimport set Reference to bz30557.
bzimport added a subscriber: Unknown Object (MLST).

heylarson wrote:

I should also point out that the categories that do manage to appear in the treeview do not actually expand or collapse. Pressing the [+] has no effect.

The problem is caused by the fact that the generated HTML is invalid, for example you'll get

<ul id="SelectCategoryList"></li>  (yes, <___/___li>!)

at the beginning and a wrong order and count of </li> and </ul> tags.

I'm working on a patch, but... (to make it less boring ;-)
a) I'm using an old version from the REL_1_16 branch
b) I have other fixes in the same code section (see my bugreports for SelectCategory), which means I probably won't be able to produce a separate patch

BTW: The interesting thing is that IE is that picky about the invalid HTML here - I'd have expected other browsers to complain about the invalid HTML much more...

Created attachment 9868
patch for this and other issues

Patch against
http://svn.wikimedia.org/svnroot/mediawiki/branches/REL1_16/extensions/SelectCategory

This patch includes fixes for the following issues:

  • fix nesting and closing of <ul>/<li> (bug 30557 = this bugreport) (if you are only interested in those changes, take all lines containing $fresh_level_started)
  • make $wgSelectCategoryMaxLevel working (bug 27795, fixed in trunk)
  • $wgSelectCategoryRoot ignored on Special:Upload (bug 24911)
  • new config option $wgSelectCategoryToplevelAllowed

attachment SelectCategory.diff ignored as obsolete

sumanah wrote:

Thanks for the patch! I've added the "need-review" and "patch" keywords so other developers know to review your patch and get back to you.

Please update your patch for the current trunk code (not a release branch, and especially not one that old).

Yes, I know that the 1.16 branch is old ;-)
Unfortunately I don't have a wiki running with 1.18.1 or trunk at the moment, so updating the patch will take some time.

In other words: If someone is faster in updating (and splitting) the patch, I won't object ;-) (hint: take all lines containing $fresh_level_started, shouldn't be too hard)

Created attachment 10844
updated patch (for latest SVN)

This patch fixes (only) the nesting of <ul> and <li> tags and their closing counterparts.

You can find the other parts that were in attachment 9868 in various open bugreports for SelectCategory (see https://bugzilla.wikimedia.org/buglist.cgi?component=SelectCategory&resolution=--- for all open bugreports)

Attached:

(In reply to comment #9)

Submitted to SVN https://www.mediawiki.org/wiki/Special:Code/MediaWiki/115705

This means this bug is fixed in SVN trunk - so let's close it as fixed ;-)

(In reply to comment #10)

This means this bug is fixed in SVN trunk - so let's close it as fixed ;-)

Well, I guess someone should review & verify... did you?

The commit exactly matches my patch (which I'm using in production since months), so I'd call it verified ;-) - and I'm used to mark bugs as fixed when the fix is in SVN or git.

If my method doesn't match the MediaWiki workflow, pointers to the correct workflow are welcome ;-)

(In reply to comment #12)

The commit exactly matches my patch

Sounds like reviewed & verified to me :)

Re workflow: for git we are using a pre-commit review process, so something that is merged there can be considered fixed. With SVN, that is not necessarily the case, and there's no easy way to tell. That's one of the reasons we switched to git/gerrit.

Anyway, thanks for looking into it!