Page MenuHomePhabricator

Tablesorter not sorting certain columns in Firefox & Safari
Closed, ResolvedPublic

Description

On https://nl.wikipedia.org/wiki/Wikipedia:Regelingen_rond_moderatoren/Activiteit_moderatoren sorting doesn't work for the second column. The first and the third are fine, on the second column nothing happens apart from the sorting arrow changing.

This happened on Firefox on MacOs when logged in, but also on Safari on the same Mac, not logged in.

Related Objects

Event Timeline

Akoopal created this task.Feb 13 2016, 8:44 PM
Akoopal raised the priority of this task from to Needs Triage.
Akoopal updated the task description. (Show Details)
Akoopal added subscribers: Akoopal, Krinkle.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptFeb 13 2016, 8:44 PM
Krinkle renamed this task from table not sorting. to Tablesorter not sorting certain columns in Firefox.Feb 13 2016, 8:47 PM
Krinkle updated the task description. (Show Details)
Krinkle set Security to None.
Krinkle added a subscriber: TheDJ.

I can reproduce this on Firefox 43 (Mac). However the bug does not happen in Chrome.

There are no JavaScript errors in the console when the bug happens in Firefox. It just doesn't change rows and remains silent.

TheDJ added a comment.Feb 13 2016, 9:27 PM

It seems that is recognizes the contents as isoDate, but then isn't able to complete the sorting for some reason. If you specify data-sort-type="text" for the column, it sorts as plain text..

Firefox needs a "T" in strings for isoDate:

new Date( "2010-02-06T19:12:00" ).toISOString();

is ok.
I have a solution for this - but it's today not ready for commit. See mw:Tablesorter/Improved parsers ...

It doesn't seem to sort in Safari too.

Danny_B renamed this task from Tablesorter not sorting certain columns in Firefox to Tablesorter not sorting certain columns in Firefox & Safari.Feb 16 2016, 8:17 AM
TheDJ added a comment.EditedFeb 16 2016, 10:54 AM

Firefox needs a "T" in strings for isoDate:

Ah. this is logical. I made the format 'detection' regex relatively lenient on purpose, but apparently the Date parsing in browser implementations expects a more strict format and doesn't allow for all the intricacies of actual iso dates (which is probably a wise decision, since iso dates can be a bit bonkers).

We can easily fix the format regex to be stricter and then it could fallback to non iso date or text parsing. Long term I would like us to separate --data-sort-type into a type and a format btw. Currently it's not possible to mix dates and isodates, which doesn't seem like a good thing.

I added a lot of test cases for the parsers last year, so any of @MatthiasDD's proposals will be much easier to incorporate, but I do advise you to separate everything into very distinct patches.

For the specific page, which is bot generated, I asked the bot-operator to just make it a proper isodate by adding a T. For a short-term fix, I think that indeed the regexp needs to be stricter.

Change 287449 had a related patch set uploaded (by MatthiasDD):
jquery.tablesorter: Improve detection and handling of isoDate

https://gerrit.wikimedia.org/r/287449

Change 287449 merged by jenkins-bot:
[mediawiki/core@master] jquery.tablesorter: Improve detection and handling of isoDate

https://gerrit.wikimedia.org/r/287449

matmarex closed this task as Resolved.Aug 13 2017, 6:47 PM
matmarex removed a project: Patch-For-Review.