Page MenuHomePhabricator

[Regression] style="background:XXXX;" in headers breaks sortable tables
Closed, DeclinedPublic

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
|-
|John||Smith||1.85
|-
|Ron||Ray||1.89
|-
|Mario||Bianchi||1.72
|-class="sortbottom"
|||Average:||1.82
|}

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

Details

Reference
bz31755

Event Timeline

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

r98069, r98665 and r99307 mostly deal with this problem.

lexein-w wrote:

When will the fix be rolled out?

(In reply to comment #2)

When will the fix be rolled out?

There is no schedule for that.

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.

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" |NameSurnameHeight

-
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

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

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.

"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.

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.

(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

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.

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.

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

(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.

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.

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.

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.

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.

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.

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.

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.

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.

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

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.

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.

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.

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.

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.

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

Krinkle set Security to None.
Krinkle unsubscribed.

If not, is there a workaround?

The workaround is to use background-color: XX instead of background: XX. The problem with background is that it is a shorthand rule that represents all background-* related CSS properties. Including background-image, which is what jquery-sortable uses to render the clickable sort icon.

By overriding all background rules in the wikitext, it removes the sort icon.

Considering that it has been several years, and there is a workaround, and solving it will cause more problems, declining this bug.

The added problem here is that there is no way to infer what the editor intended. background is a CSS rule to override all other background rules. That is the semantic intent associated with that rule. If we would break that contract by forcefully showing the sort icon anyway, we'd instead get a bug report like "Sort icon override no longer works" or "Sort icon shows up unexpected when custom background image was intended" or something like that.

@Krinkle could the wikitext-parser solve this issue?
If I am right, from wikitext only bqcolor is useable. Any junk behind disable backgroud-attribute. If the parser split background into it's atoms, and send only background-color to parasoid this issue should be solved.