Last modified: 2012-12-03 15:41:25 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 T43886, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 41886 - [regression] tablesorter doesn't handle headers with colspan properly
[regression] tablesorter doesn't handle headers with colspan properly
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
All All
: High critical (vote)
: ---
Assigned To: Nobody - You can work on this!
: code-update-regression
Depends on:
Blocks: 31601
  Show dependency treegraph
Reported: 2012-11-08 13:07 UTC by Derk-Jan Hartman
Modified: 2012-12-03 15:41 UTC (History)
11 users (show)

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


Description Derk-Jan Hartman 2012-11-08 13:07:41 UTC
We now always do explodeRowspans before the first sort, to accomodate the new .sort() function. This has the unintended consequence that you basically cannot use rowspans at all anymore, since the JS code will always immediately fragment them back into individual cells right now.

This is a side effect of:
Comment 1 Oliver Keyes 2012-11-09 11:56:00 UTC
If I'm understanding this right, this has essentially broken rowspan? Given the sheer number of pages this has altered I'm upping the priority - I'll try to give the bugmeister a kick.
Comment 2 Andre Klapper 2012-11-09 12:30:26 UTC
Krinkle, Sam: Can you take a look at this, please?

High priority: sortable && rowspans don't work together at all, pretty visible for readers.

Example with "sortable" still enabled:

Potentially related past code changes (quick check): Update jQueryUI to 1.9.1 make $.tablesorter treat alt like text

Copying from the thread on :

"It appears that clicking the nth column header actually did sort the nth column, but the first header has colspan="3". For all subsequent headers this means that the column being sorted is two columns to the left of the clicked header. Any chance the table sorter could be modified to handle this?" PrimeHunter 14:14, 8 November 2012 (UTC)

"What is the logic about a colspan="3" on a header, and then saying that the column should be sortable, while that column actually contains 3 columns per row.. There are so many 'implied' assumptions being made by authors in such a process, it's difficult to explain that to a JS. Also probably not that 'accessible' to screenreaders btw. Perhaps we should just avoid such constructs on tables." —TheDJ 14:27, 8 November 2012 (UTC) 

Note to myself: The previous issue for this example when values were shown in wrong table cells (making the table completely unreadable) was a markup issue: Using ! instead of | in this specific table:
Comment 3 Sumana Harihareswara 2012-11-09 15:16:50 UTC
I agree that this is not HIGHEST priority, because there's no data loss, but it's pretty high.  CC'ing Hoo & Matmarex to see whether they can provide a quick JS fix.

If lots of tables across the wikis are mistakenly using ! where they should have used |, and MediaWiki used to be forgiving and now it's not and the tables croak, we should address that (either with an educational campaign, a regex, or building some kind of forgiveness into the rendering).
Comment 4 Sumana Harihareswara 2012-11-09 15:17:07 UTC
But that last item (! instead of |) would be a separate bug.
Comment 5 Oliver Keyes 2012-11-09 15:24:18 UTC
Yeah. If (as I understand it) the VE will standardise formatting on save, it might be something worth temporarily punting on and asking them to sort out if/when they deploy.
Comment 6 Andre Klapper 2012-11-09 15:57:24 UTC
(In reply to comment #4)
> But that last item (! instead of |) would be a separate bug.

-> bug 41889
Comment 7 Bartosz Dziewoński 2012-11-09 15:58:04 UTC
Changing description from "tablesorter explodes rowspans now before ever
clicking sort" to "tablesorter doesn't handle headers with colspan properly",
as I understand that this is the real problem.

I'll try to look into it.
Comment 8 Bartosz Dziewoński 2012-11-09 16:54:11 UTC
Fixed in I89766c96. The headers now sort on all the columns they span over.
Comment 9 Brad Jorsch 2012-11-09 19:51:25 UTC
Gerrit change #32593 fixes the original bug reported here (rowspans being exploded automatically).
Comment 10 Andre Klapper 2012-11-14 13:46:55 UTC
This hasn't gotten in yet - could anybody do a review in gerrit?
Comment 11 Bartosz Dziewoński 2012-11-16 21:45:35 UTC
Change merged.

Should this be backported and deployed, maybe?
Comment 12 Bartosz Dziewoński 2012-11-16 22:00:12 UTC
Actually, not fixed yet; sorry for the confusion.

This bug would be fixed by change I89766c96 (mine), which isn't yet merged; the merged one, Ibe4cc7e9 (Brad's), fixes bug 41889 (which was split from this one).
Comment 13 Brad Jorsch 2012-11-19 17:54:01 UTC
(In reply to comment #12)
> Actually, not fixed yet; sorry for the confusion.
> This bug would be fixed by change I89766c96 (mine), which isn't yet merged; the
> merged one, Ibe4cc7e9 (Brad's), fixes bug 41889 (which was split from this
> one).

There are actually three issues mentioned here:
1. The original bug reported here, that tablesorter was exploding rowspans on page load instead of on first sort. That is fixed by Ibe4cc7e9.
2. The bug this transformed into, that tablesorter doesn't sort correctly when headers are colspanned. That is fixed by I89766c96, once someone merges it.
3. Bug 41889, that tablesorter gets cells out of order when row header cells are involved. I just submitted Icb674f7e to fix that one.
Comment 14 Andre Klapper 2012-11-26 16:27:04 UTC
Brad: Thanks for the analysis!

The patches for 1. and 2. are merged, so only Icb674f7e for bug 41889 is left.
Comment 15 Brad Jorsch 2012-11-26 16:30:11 UTC
Since 1 and 2 are both merged now, I'm going to close this bug. Bug 41889 is still open for that issue.

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