Bug 16077 – XSLT processor <xsl:sort> algorithm is incompatible with other browser implementations

• Status: RESOLVED
• Resolution: FIXED
• Component: XML

[darren]

When sorting an XML dom using an XSL transformation  (via the <xsl:sort>)  the algorithm doesn't appear to be to spec:

http://www.w3.org/TR/xslt#sorting

from the spec:
case-order has the value upper-first or lower-first; this applies when data-type="text", and specifies that upper-case letters should sort before lower-case letters or vice-versa respectively. For example, if lang="en", then A a B b are sorted with case-order="upper-first" and a A b B are sorted with case-order="lower-first". The default value is language dependent.
....

Safari appears to be sorting a list with upper and lower case entries in it like this

  A B a b

rather than like this

 A a B b  *or* a A b B (depending on the value of the case-order attribute).

For a test case, download the XML and XSL files on this page:
   http://www.w3schools.com/xsl/xsl_sort.asp

... then edit the entries to include an artist with all lower-case chars.  Display this in Safari,
FF, and IE to see the difference.

[Maciej Stachowiak]

A simple standalone reduction based on that example would be helpful.

[darren]

Created attachment 17423 [details]
XML file to Sort

XML file to sort

[darren]

Created attachment 17424 [details]
XSL file to do sorting transformation

XSL file to do sorting transformation.

[darren]

To Repro:

1. Download the XML and XSL files attached.
2. Load them In the browser:

Result:
sorted as:
  Angie
  Brad
  abby
  brent

Expected:
sorted as:
  abby
  Angie
  Brad
  brent

[Alexey Proskuryakov]

I have searched libxslt bugs, and it appears to be a well known issue that they refuse to fix: <http://bugzilla.gnome.org/show_bug.cgi?id=143383>.

It's possible to override the broken default implementation with xsltSetCtxtSortFunc().

[Maciej Stachowiak]

<rdar://problem/5609785>

[Alexey Proskuryakov]

Created attachment 17433 [details]
proposed fix

[Eric Seidel (no email)]

Comment on attachment 17433 [details]
proposed fix

I would think that xml/XSLTUnicodeSort.cpp should be called xml/XSLTUnicodeSortICU.cpp, or at least the ICU parts abstracted out into another file.  Otherwise we're just going to get lots of #ifdefs when Qt comes and adds their implementation, etc.

Otherwise the change looks fine.

[Alexey Proskuryakov]

I agree that we will eventually need to abstract out collation from here - but I feel that it's something that needs to settle down a bit. Immediately introducing abstractions when adding a new concept sometimes gives poor results due to incomplete understanding of needs.

[Maciej Stachowiak]

Comment on attachment 17433 [details]
proposed fix

It's unfortunate that we have to cut & paste so much XSLT code, but I guess there's no better way. Too bad you can't just provide a string compare function for it to use.

r=me

[Alexey Proskuryakov]

Committed revision 27984.