894874 - document.selectedStyleSheetSet erroneously reports empty string

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Steps to reproduce the bug:

1. open http://www.w3.org/Style/Examples/007/alternatives.en.html
2. open the Browser console
3. use View > Page Style to select an alternate stylesheet
4. type    getBrowser().contentDocument.selectedStyleSheetSet
   in the Browser console to see if the name of the selected style set
   is correctly reported

EXPECTED RESULT: selected style set is always correctly reported

ACTUAL RESULT: result is correct for style sets "Main", "Cascade",
               "Cascade (older browsers)", "Draft 3", "Draft 2" and
               is the empty string for all other alternate style sets
               defined by the document: "Oldstyle", "Modernist", "Midnight",
               "Ultramarine", "Swiss", "Chocolate", "Traditional", "Steely",
               "Forest (by David Baron)" and "Plain (by David Baron)"

I have no idea if this is related or not, but the <link> elements for the
style sets reporting an empty string use an absolute URL while the ones
reporting correctly the style set's title use a relative one.

Tested with Nightly 25.0a1 (2013-07-16, built from http://hg.mozilla.org/mozilla-central/rev/5976b9c673f8)

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

FWIW, the View > Page Style menu reports correctly the selected style set
because it's not at all based on the API mentioned above.

Comment 2

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: proposed fix, tested in BlueGriffon

Ok, the guilty code is in nsIDocument::GetSelectedStyleSheetSet. The URL
referenced above links a stylesheet between the ones reporting
correctly the styleset and the one erroneously reporting it and that
sheet has no title attribute. But the code does not deal with that case
and returns an empty title...

Comment 3

[Boris Zbarsky [:bzbarsky]]

No, that patch is wrong.

This is a regression from bug 804631.  The relevant bogus part of that diff:

     if (aSheetSet.IsEmpty()) {
       aSheetSet = title;
-    } else if (!title.IsEmpty() && !aSheetSet.Equals(title)) {
+      return;
+    }
+
+    if (!title.IsEmpty() && !aSheetSet.Equals(title)) {

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix .selectedStyleSheetSet to consider all the sheets even after it finds an enabled sheet.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 777131 [details] [diff] [review]
Fix .selectedStyleSheetSet to consider all the sheets even after it finds an enabled sheet.

Odd API

Comment 6

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/551ccbf6c251

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/551ccbf6c251

Comment 8

[Kohei Yoshino [:kohei]]

I was about to report this regression. Could you fix it also in Gecko 24?

Comment 9

[Boris Zbarsky [:bzbarsky]]

It's a simple enough backport.  I didn't worry about it too much because it had been shipping for something like 3 releases already (20, 21, 22) and this API is not exactly well-supported in browsers so doesn't affect web compat much (clearly, since we shipped it for several releases...).

Feel free to request branch approval and make a case for it, though.

Comment 10

[bhavana bajaj [:bajaj]]

Not a tracking bug given its an old regression, we may take an uplift to beta depending on the risk evaluation and the Beta timeline.