[Regression] style="background:XXXX;" in headers breaks sortable tables
OpenPublic

Description

Author: lexein-w

Description:
Any styling in table headers disables sorting for that column.
Example:
{|class="wikitable sortable"
!style="background:#cfcfcf;" align="center" |Name!!Surname!!Height

-
JohnSmith1.85
-
RonRay1.89
-
MarioBianchi1.72
-class="sortbottom"
Average:1.82
}

Is this fixed in 1.19?
If not, is there a workaround?


Version: 1.18.x
Severity: normal

bzimport added a subscriber: Unknown Object (MLST).
bzimport set Reference to bz31755.
bzimport created this task.Via LegacyOct 16 2011, 4:55 PM
TheDJ added a comment.Via ConduitOct 22 2011, 4:51 PM

r98069, r98665 and r99307 mostly deal with this problem.

bzimport added a comment.Via ConduitOct 22 2011, 9:49 PM

lexein-w wrote:

When will the fix be rolled out?

TheDJ added a comment.Via ConduitOct 22 2011, 10:13 PM

(In reply to comment #2)

When will the fix be rolled out?

There is no schedule for that.

MarkAHershberger added a comment.Via ConduitOct 23 2011, 8:43 PM

I've made sure that the revisions DJ referred to in comment 1 are marked for deployment and will talk to someone tomorrow about pushing this out.

CCing Roan and Reedy on this since they're most likely the ones who will push it.

Timeshifter added a comment.Via ConduitNov 14 2011, 5:48 PM

Adding some blank headers above the styled headers fixes the sorting problem. It also narrows the table, which is very useful at times. Here is the same table after it has been fixed:

{|class="wikitable sortable"

-

! <br>
! <br>
! <br>

-

!style="background:#cfcfcf;" align="center" |Name!!Surname!!Height

-
JohnSmith1.85
-
RonRay1.89
-
MarioBianchi1.72
-class="sortbottom"
Average:1.82
}

See several sortable tables with styled headers here too:
*http://en.wikipedia.org/wiki/List_of_Occupy_movement_protest_locations

Timeshifter added a comment.Via ConduitNov 14 2011, 8:14 PM

For more info see the header sections of this help page about table sorting:
*http://en.wikipedia.org/wiki/Help:Sorting

Adding some blank headers above the named headers also allows more space for the names within header cells, since no space is taken up by the sorting icon. This is important for tables with many columns. For example; see this table with many columns:
*http://en.wikipedia.org/wiki/List_of_countries_by_income_equality

TheDJ added a comment.Via ConduitNov 14 2011, 9:03 PM

Reopening, since !important change was reverted.

We need to rewrite the sortable tables to make the indicator a separate element that is not easy to style for normal people.

TheDJ added a comment.Via ConduitNov 14 2011, 9:09 PM

"Adding some blank headers above the styled headers fixes the sorting problem."

That is not the proper way btw. Simply don't overwrite the background style. In this case, use "background-color" instead for instance.

tstarling added a comment.Via ConduitNov 14 2011, 10:32 PM

I'm recommending that we re-add the !important change (r99307) in the 1.18 branch only, since it's the short-term option that gives us the least amount of breakage. Leaving it how it is is a fair bit worse in terms of user impact.

In trunk maybe we can have something based on adding <img/> tags, that's probably the most robust solution.

Reedy added a comment.Via ConduitNov 15 2011, 12:48 PM

(In reply to comment #9)

I'm recommending that we re-add the !important change (r99307) in the 1.18
branch only, since it's the short-term option that gives us the least amount of
breakage. Leaving it how it is is a fair bit worse in terms of user impact.

In trunk maybe we can have something based on adding <img/> tags, that's
probably the most robust solution.

Done in r103156

Timeshifter added a comment.Via ConduitNov 17 2011, 1:17 AM

Concerning comment #8: "Simply don't overwrite the background style. In this case, use 'background-color' instead for instance."

Is it realistic to expect people to figure out that they should use style="background-color:" versus style="background:" in header cells?

I hope that the fix to come allows any method of styling for header cells. Also, I hope that the sorting icon is put above the header name so that tables
with many columns have more room for the header text.
*http://en.wikipedia.org/wiki/List_of_countries_by_income_equality

Then there would be no need for an extra row of blank headers. In any case styling is essential for header cells in some cases.

bzimport added a comment.Via ConduitNov 17 2011, 1:48 AM

lexein-w wrote:

(In reply to comment #11)

Concerning comment #8: "Simply don't overwrite the background style. In this
case, use 'background-color' instead for instance."

Is it realistic to expect people to figure out that they should use
style="background-color:" versus style="background:" in header cells?

Yes, if they look at Help:Sorting, where I've added the background-color tip there as "Workaround 1", and added back a mild caution for Workaround 2.

Timeshifter added a comment.Via ConduitNov 24 2011, 7:33 AM

Reflinks is converting style="background-color:" to style="background:" in header cells. See this diff:
*http://en.wikipedia.org/w/index.php?title=List_of_Occupy_movement_protest_locations_in_the_United_States&action=historysubmit&diff=462226262&oldid=462222535

Fortunately, the table in that article uses both workarounds discussed in Help:Sorting. Otherwise Reflinks would have removed sorting from that table.

So people following the style="background-color:" workaround in Help:Sorting are being undermined by Reflinks. Is there some logical reason why Reflinks is doing what it is doing? Reflinks:
*http://en.wikipedia.org/wiki/User:Dispenser/Reflinks

Catrope added a comment.Via ConduitNov 24 2011, 10:35 AM

(In reply to comment #13)

So people following the style="background-color:" workaround in Help:Sorting
are being undermined by Reflinks. Is there some logical reason why Reflinks is
doing what it is doing? Reflinks:
*http://en.wikipedia.org/wiki/User:Dispenser/Reflinks

This is probably just a bug in reflinks, I suggest you contact the author about it.

Timeshifter added a comment.Via ConduitNov 27 2011, 1:15 AM

The sorting icon really needs to be above the text in a header cell. This allows for more columns to fit within the width of the screen. It also allows more space for the text within the header cells, since no space is taken up by the sorting icon. This is useful for tables with many columns. For example; see
*http://en.wikipedia.org/wiki/List_of_countries_by_intentional_homicide_rate

Go to that page and make your browser window more narrow (the middle button of the 3 buttons at the top of the browser window). Drag the right side of the window to get various window widths. Scroll down the page. Note that when the sorting icon is to the right of the text that the sorting icon does not wrap. So the table then requires a horizontal scrolling bar sooner than the tables with the sorting icons above the headers.

Also, note the tables in the "Country subdivisions" section of that page. The tables start off closed. I can not put the sorting icons above the text. If I do then the column header text is not readable. The text would be readable if the default position of the sorting icon was above the text. Right now there is no way I can fix this problem.

bzimport added a comment.Via ConduitNov 27 2011, 2:23 AM

lexein-w wrote:

This nonstandard format (sorting icon above the text) doesn't "need to be" anything until a wide consensus of editors, and the Mediawiki coders agree that it "needs to be". Cramming data into the minimum possible space is not the primary purpose of tables, or sortable tables.
Rather than continue to argue this here, try the Village Pump (technical), or one of the MOS talk pages.

Timeshifter added a comment.Via ConduitNov 27 2011, 3:44 AM

You know, common sense is allowed according to Wikipedia guidelines. Adjusting the code to allow styling in headers is going to be adjusting code for the sorting icon too. Because clicking the header cell activates sorting. Currently, the code has to do gymnastics to distinguish between clicking links in a header cell versus clicking anywhere else in the header cell to activate sorting. See the examples here:
*http://en.wikipedia.org/wiki/Help:Sorting#Header_styling.2C_links.2C_and_markup

Click within the header cells in those examples. Rather than use such complicated code it is a lot easier to put the sorting icon above the text, and then the only thing that is clicked to activate sorting is the sorting icon. You don't need a committee to use common sense.

TheDJ added a comment.Via ConduitNov 28 2011, 1:30 PM

Please remember that wikipedia is not the sole usecase of mediawiki. If having sort headers ABOVE normal headers was a good idea, then likely it would work like that in every data tool in the world. It doesn't, which seems to indicate that there is more in play normally, then just cramming as much information into one table as possible.

And again, it is an unintended consequence of the implementation that it works. People should not rely on it to exist like that for perpetuity, it might break at any time.

Timeshifter added a comment.Via ConduitNov 28 2011, 4:47 PM

I don't want the sorting icon to be in a separate cell. I want it to be in the same cell as the header text. But above the text. It is simple to do with a line-break. <br>

I looked at other table sorting:
*https://www.google.com/search?q=table+sorting
None of them have our problem of reference links and internal links in header cells. Very few popular sites that are read mostly by laypeople have clickable references throughout articles. That is a MediaWiki and Wikipedia thing. And we put the clickable references in header cells too. Along with internal links to other Wikipedia articles.

bzimport added a comment.Via ConduitNov 28 2011, 5:27 PM

lexein-w wrote:

(In reply to comment #19)

I don't want the sorting icon to be in a separate cell. I want it to be in the
same cell as the header text.

This should be discussed not here, but at the equivalent of the MediaWiki village pump. Seriously.

Timeshifter added a comment.Via ConduitNov 28 2011, 6:31 PM

I looked around at http://www.mwusers.com and http://www.mediawiki.org a little and did not find any place for discussing technical bugs. The most skilled people are here.

bzimport added a comment.Via ConduitNov 28 2011, 6:34 PM

sumanah wrote:

zlight, try https://lists.wikimedia.org/mailman/listinfo/wikitech-l and https://lists.wikimedia.org/mailman/listinfo/mediawiki-l which have much larger readerships. Here on Bugzilla, only 8 people are looking at this bug.

Timeshifter added a comment.Via ConduitNov 28 2011, 8:45 PM

lexein, I think you should initiate discussion here:
*https://lists.wikimedia.org/mailman/listinfo/wikitech-l.
There are over a thousand Mediawiki bugs open according this page:
*https://bugzilla.wikimedia.org/weekly-bug-summary.cgi?tops=10&days=7
I think the most we can expect from posting to the wikitech list is to get a few more people to look at this bug 31755 thread. I do not understand why the location of the sorting icon above the text as opposed to the right side of the text matters to you other than that it is different. So I think you should explain your reasoning to the wikitech list.

Derk-Jan Hartman, I think you may have misunderstood what I was proposing. You wrote in comment 18: "And again, it is an unintended consequence of the implementation that it works. People should not rely on it to exist like that for perpetuity, it might break at any time." I am not asking for a separate cell for the sorting icon. I am asking that the sorting button-icon be moved above the text but remain in the same cell. If the sorting icon is in the same cell (whether it is above or to the right of the text) then it will not break.

I could give more examples of why putting it above the text prevents many problems. Mainly, people use various size fonts. I use a larger font normally in Firefox (minimum font size setting). Wikis on various sites (Wikia and elsewhere) use various content widths. Tables oftentimes do not fit well in the allocated content width. Editors oftentimes do not account for larger fonts, smaller screens, percentage width settings, and so on. Expansion of column width can occur when the column head is clicked for some of the table sorting methods found here:
https://www.google.com/search?q=table+sorting

bzimport added a comment.Via ConduitNov 28 2011, 11:17 PM

lexein-w wrote:

Excuse me, but there's no need for me to initiate anything. You're the one insisting on a MediaWiki-wide feature change which will affect hundreds of thousands of Wikipedia pages with sorting tables. Feature changes and styling changes get discussed elsewhere, not in the middle of a bug report. My report was of a NONFUNCTIONING SORT COLUMN due to styling. I just want the documented feature to work as documented, if a bit of styling has been added.

Catrope added a comment.Via ConduitNov 30 2011, 12:57 PM

The workaround for this bug is to use specific background styling: style="background-color:#123456;" will work just fine. It's not "any" stying that disables the sorting icon, it's using the catch-all background: attribute, which overrides the background-image: set by the table sorter.

Regarding the placement of the sorting icon (above or next to the header): if you want to discuss changing that, open a separate bug for it or start a thread on wikitech-l. Let's keep this bug on topic.

Timeshifter added a comment.Via ConduitNov 30 2011, 2:25 PM

I do not feel this requires a separate discussion. If you feel it does, then do so. Whoever is working on this can put the sorting icon above the text. It is not a big deal. This thread is about separating styling from sorting. Making the sorting happen in the most logical way, clicking the sorting icon, further separates sorting from header styling. This is not a massive change, nor illogical. It is simple placement. If it bothers you, or others, then one solution is to add a feature to choose where to place the sorting icon. Sort-top, Sort-right, Sort-bottom, Sort-left. It is a mistake in my opinion to make sorting happen by clicking text. By the way, Roan, I left a message to the Reflinks author, and have not heard anything. Another reason to make sorting happen only via the sorting icon. Because if various bots are changing header styling, then sorting will get broken.

TheDJ added a comment.Via ConduitDec 19 2011, 9:13 AM

The discussion is just the reason why it needs to be a separate bugreport. Your comments are distorting the clarity of this bug report that deals with JUST one thing. "When a user manually sets a background style on a table headercell, the sorting indicator will disappear". That is the only thing this bug deals with and the fix for that will be the basis for any future closing of this bugreport.

I have opened bug 33249 for your problem, please continue there.

Krinkle added a comment.Via ConduitMar 2 2012, 9:55 PM

Summary:

  • Up until the point where jquery.tablesorter replaced the wikibits tablesorter code, the "sort" trigger icon was an <img> element injected into the <th> element. No CSS was applied by default by the tablesorter
  • Wikis commonly used class=wikitable or inline style="" attributes for the styling of the actual table being sorted
  • The jquery.tablesorter module now applies CSS:
    • a background-image on the <th>
    • padding on the <th> (to avoid text from continuing where the image appears)
    • background-position on the <th>

The way jquery.tablesorter does it is imho a more ideal approach, and easier to customize and maintain.

However since it isn't a new module but replacing an old one, we need to think about compatibility with the 1000s of articles out there written with thought (or need to think of) that some other CSS was going to be applied later on.

Wikis didn't have to account for styling before, so there are 10,000s of articles all over the place with inline hardcoded style attributes on tables. Including generic catch-all style rules like "background: red;" that unintentionally override background-image and background-position of the jquery.tablesorter, thus causing the sortable table to no longer have an icon visible (A UX problem, and a regression as well since the icon was perfectly visible in the old system.

Couple of solutions come to mind, in no particular order:

A) We somehow enforce the styling (!important)

  • Advantage: Only re-enforces the properties we care about, other inline styles should be fine
  • Disadvantage: We're using !important, so local wikis will have a hard time customizing anything, as well as other modules from core/extensions or gadgets (e.g. a custom sortable icon)

B) We get rid of the css part of jquery.tablesorter targeting the <th> and instead go old-school and put the "sort" icons as actual child elements of the <th> (doesn't have to be <img>, could be a <span> with css)

  • Advantage: No breaking changes for existing articles
  • Disadvantage:

C) We keep the current module in it's "ideal" state with css and all, but instead of targeting the old "sortable" class, we use a new class "mw-tablesorter" or something as the "new" way. That way there is no breaking of anything. We then also implement a B)-like solution for backwards compatibility with the old system.

RobLa-WMF added a comment.Via ConduitMar 14 2012, 4:58 PM

Since this was a problem in 1.18, we won't be breaking anything new releasing this in 1.19. Adjusting target milestone accordingly.

MarkAHershberger added a comment.Via ConduitSep 30 2012, 6:04 PM

still broken, still bumping.

Add Comment