Last modified: 2013-07-20 20:50:50 UTC
Created attachment 5244 [details] unified diff file This patch adds the ability to force a particular sortorder of a column instead of relying on the script to detect the proper sortorder. This is achieved by adding a "sortorder" attribute to the column header, and specifying the desired sortorder as the value of this attribute. The actual options, however, need to be updated to be current with the other patches. The ones included in the diff can be considered examples used more for demonstration purposes.
sortorder is not a valid XHTML attribute. Adding this patch will go against our attempts at keeping MediaWiki XHTML valid. I recommend finding a way to do this using classes rather than invalid XHTML attributes.
(In reply to comment #1) > sortorder is not a valid XHTML attribute. Adding this patch will go against our > attempts at keeping MediaWiki XHTML valid. > I recommend finding a way to do this using classes rather than invalid XHTML > attributes. The proposal could certainly be changed to use classes. However, the code already uses a non standard XHTML attribute, "sortdir". Should this be changed as well?
Sortdir is an attribute set internally by the js when you click on the sort button. It's ok for JS to set non-standard attributes. There is another proposal for a default reverse sort, that patch should probably make sure to be one that sets that attribute.
It's silly to say that non-standard attributes are supported when added by JavaScript, but not otherwise. You're confusing being able to pass a verification test with the issue behind the proviso.
The sortdir thing should be changed too. Frankly I don't see why authors shouldn't be allowed to make up their own site-specific attributes. It's handy sometimes. But we want the trendy Standards-Compliant label, I suppose, so we have to live with it. There's no way, in any event, for users to add a custom attribute in wikitext. Something like this: {| class="sortable" |- ! sortorder="numeric" | This is the header ... |} will just have the "sortorder" attribute stripped as invalid. So I'm not clear on how this is supposed to be used, even if we did want to break XHTML validity, which we don't.
It's only used internally. Attributes in the code are commonly used for storing variables related to a node. In this case we store the current sort direction so that we can reverse it on the next sort. I can't remember if we have a class to make the list initially sorted... If we don't it would be good. Either way a reversesort css class to make this sort in the opposite direction by default would be nice.
I took a look at the code to see how easy it would be to accomplish this using only class names. While it could be done, it would require supporting a separate classname for each sorting function. As the number of sorting functions increases, so does the number of classes you have to check for (as opposed to checking for a single, uniquely named attribute). I'm worried that the large number of classes will negatively affect the speed of the script.
You don't have to check for a pile of stuff, just a pattern. var sortorder
Ack, wtf, I write a section of JS code, and bugzilla deletes it. Bah... Well, really you'd just break up the className by \s and then test for any classNames that start with something like sortorder-
You mean, like: "sortorder-numeric" "sortorder-alphanumeric" "sortorder-date1" ... I suppose that would work.
Created attachment 5263 [details] unified diff file I created an alternate version that uses class names instead of attributes. Seems to work OK.
Actually, "sortorder-XXXX" could be truncated to "sort-XXXX" without any problems.
Created attachment 5266 [details] unified diff file "sortorder-xxxx" classes renamed to "sort-xxxx".
Should the regex be anchored? I think something like var sortClass = ('' + td.className + '').match(/^sort-\w+$/); would be more appropriate
(In reply to comment #14) > Should the regex be anchored? I think something like > var sortClass = ('' + td.className + '').match(/^sort-\w+$/); > > would be more appropriate > Oops, that would probably break multiple classes....
(In reply to comment #15) > (In reply to comment #14) > > Should the regex be anchored? I think something like > > var sortClass = ('' + td.className + '').match(/^sort-\w+$/); > > > > would be more appropriate > > > Oops, that would probably break multiple classes.... You just made me notice that the quotes are supposed to have spaces in them. I.e. - var sortClass = ('' + td.className + '').match(/(sort\-\w+)/); + var sortClass = (' ' + td.className + ' ').match(/(sort\-\w+)/); -Mike
(In reply to comment #16) > (In reply to comment #15) > > (In reply to comment #14) > > > Should the regex be anchored? I think something like > > > var sortClass = ('' + td.className + '').match(/^sort-\w+$/); > > > > > > would be more appropriate > > > > > Oops, that would probably break multiple classes.... > > You just made me notice that the quotes are supposed to have spaces in them. > I.e. > > - var sortClass = ('' + td.className + '').match(/(sort\-\w+)/); > + var sortClass = (' ' + td.className + ' ').match(/(sort\-\w+)/); > > -Mike > Then should the regex have the spaces, too? i.e. var sortClass = (' ' + td.className + ' ').match(/ (sort\-\w+) /); It seems like that should avoid false positives in the match.
*** Bug 13535 has been marked as a duplicate of this bug. ***
(In reply to comment #17) > Then should the regex have the spaces, too? i.e. > var sortClass = (' ' + td.className + ' ').match(/ (sort\-\w+) /); > It seems like that should avoid false positives in the match. I suppose it can't hurt. I don't think false positives are very likely though. -Mike
How come this has stalled for so long?
will be fixed in the jQuery implementation
fixed in patch for Bug 8028
Quote: "How come this has stalled for so long?" Because I got into an argument with the devs and they said they would no longer be looking at my requests.
r86088
r86088 is seriously broken and may get reverted; reopening. Needs unit tests (see initial basic unit tests in r90595 which show up errors; will also need tests for the particular case this bug is about.)
reclosing
This needs to be implemented soon. It is very difficult to follow all the rules in this and other sections of the Help:Sorting page. Even experienced table editors like myself have difficulty. There are so many rules, and they change at times: *http://en.wikipedia.org/wiki/Help:Sorting#Numerical_sorting_problems I have been updating Help:Sorting, and doing various tests. There are still some inexplicable bugs where a column follows all the rules for numerical sorting, and it still does not sort correctly. I have looked at r86088 comments, and I am glad there has been some serious work down. Has work on this stalled? If you are working on this then please also incorporate my request to put the sorting icon above the text. This is very important. See my comment #15 here: *https://bugzilla.wikimedia.org/show_bug.cgi?id=31755#c15
See this related feature request: Bug 24523 - "Make table sorting ignore refs when determining data type".
This has been fixed in English Wikipedia according to discussion here: *[[Help_talk:Sorting#HTML5]] *[[Help_talk:Sorting#Documentation]] data-sort-type plus a value forces a column to be sorted according to a specific method. See also: *[[Help:Sorting#Numerical_sorting_problems]] *[[meta:Help:Sorting#Sort_modes]] - go to the subsection about forcing the sort mode for a column. It lists the values that can be used.
I think we can call this one closed now.
