Closed Bug 200930 Opened 21 years ago Closed 17 years ago

[FIX][AltSS] Implement scriptable method of choosing a stylesheet set off the document object (altss dom)

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, Whiteboard: Note: see comment 87 and comment 90 for tests)

Attachments

(4 files, 4 obsolete files)

This would allow our own "use style" impl to be much simpler (it'll still have
to deal with frames, but....)
Priority: -- → P2
Target Milestone: --- → mozilla1.5beta
Depends on: 200931
Here is the spec for what Safari implemented, in terms of a CSS3 proposed
extension. This is copied straight from an e-mail I sent to w3c-css-wg.


// Introduced in DOM Level 2:
interface DocumentStyle {
  readonly attribute StyleSheetList   styleSheets;

  // Introduced in DOM Level 3:
           attribute DOMString        selectedStylesheetSet;
  readonly attribute DOMString        preferredStylesheetSet;
};

Attributes

    selectedStylesheetSet of type DOMString

        This attribute indicates which stylesheet set (see [[HTML4]]) is in use. 
        Setting this attribute changes the 'disabled' attribute on each 
        StyleSheet object in the styleSheets object, so that all those whose 
        title matches the selectedStylesheetSet or who have no title will be 
        enabled, and all others will be disabled.
 
        selectedStylesheetSet returns the empty string if stylesheets from 
        different sets are enabled, or if all stylesheets are disabled, or if 
        there are no alternate stylesheets.

        The initial value of selectedStylesheetSet may be different from 
        preferredStylesheetSet if the UA supports persisting the 
        selectedStylesheetSet across sessions.

        Note that from the DOM's perspective, all views have the same 
        selectedStylesheetSet. If a UA supports multiple views with different 
        selected alternate stylesheets, then this attribute (and the StyleSheet 
        'disabled' attribute) return and set the value for the default view.

    preferredStylesheetSet of type DOMString, readonly

        This attribute indicates the preferredStylesheetSet as set by the 
        author. It is determined from the order of stylesheet declarations and 
        the Default-Style HTTP headers. See [[HTML4]].

-- http://lists.w3.org/Archives/Member/w3c-css-wg/2002JulSep/0073.html
Depends on: 217159
imo, selectedStyleSet and preferredStyleSet would be better names, but if this
is already out there, then I guess we'd have to stick with these.
Looks like you're making the switch depend on the style sheets' disabled
attribute. IMO style switching should be independent of that attribute.

http://lists.w3.org/Archives/Public/www-style/2003Sep/0160.html

Also, what is the intended use for lastStylesheetSet?
Yes, I am (and have always done -- see the proposal at the top of this bug, for 
instance). It has to be the case, otherwise there is no way to enable 
stylesheets that are not in the currently selected set, something that many 
pages on the Web already do, and which all browsers already support:

   http://hixie.ch/tests/adhoc/dom/css/StyleSheet/001-demo.html

I see no reason to break interoperability, especially when it is on an 
unofficial extension to the specs.

Note that one thing which my proposal _does_ change is that the 'disabled' 
attribute for alternate sheets should start off as "true", something which is 
currently not the case in all browsers, even though they are currently not used. 
Behaviour in this regard is _not_ interoperable. Opera, Mozilla, and IE all do 
different things, for instance. Mozilla defaults to "false" for disabled sheets, 
even alternate ones, while Opera and IE default to "true"; but Mozilla and Opera 
both agree that setting disabled to false should always enable the sheet, even 
if it is already false (as in Opera's case) while IE requires that the attribute 
be toggled to true first (which has no effect) then switched back to false 
(which enables the sheet). I haven't tested MacIE and Safari.


The intended use of lastStylesheetSet is to make it easy to tell what set sheets 
have to belong to to be enabled if they are dynamically inserted. See the end of 
the spec (search for "lastStylesheetSet").
> and have always done

I've only recently heard of this, so..

> something that many pages on the Web already do

Are these pages you're talking about trying to implement a style switcher
or are they just setting up alternate styles code for fun?

> Note that from the DOM's perspective, all views have the same
> selectedStylesheetSet.

If you don't mind, I could use an explanation for the non-DOM-initiate.
*bows apologetically*

> The intended use of lastStylesheetSet is to make it easy to tell
> what set sheets have to belong to to be enabled if they are
> dynamically inserted. See the end of the spec (search for
> "lastStylesheetSet").

It took me awhile to figure out what you were trying to do. It's
because enabling a style sheet from a non-selected set makes
selectedStyleSet == "", right? You need it to store the last
selection because the current situation is ambiguous wrt which
set is the selected style set.

So, if I take this link set:

<link rel="stylesheet" href="default.css">
<link rel="stylesheet" href="accent.css">

<link rel="alternate stylesheet" href="forest.css" title="Forest">
<link rel="alternate stylesheet" href="green.css" title="Forest">
<link rel="alternate stylesheet" href="green-accent.css" title="Forest">

<link rel="alternate stylesheet" href="ocean.css" title="Ocean">
<link rel="alternate stylesheet" href="blue.css" title="Ocean">
<link rel="alternate stylesheet" href="blue-accent.css" title="Ocean">

and do the following (assuming your model):
   1. The user selects Ocean from the Use Style menu.
   2. The page scripts all style sheets disabled except  forest.css
                                                    and  blue.css
   3. The page adds a style sheet with title Ocean.
   4. The page checks the selectedStyleSet: it's the empty string.
      (If the user opens the Use Style menu, nothing will be selected.)
   5. The page sets the selectedStyleSet to the empty string.
      (If the user opens the Use Style menu, Basic Page Style will be
       sepected.)
   6. The page adds another style sheet with title Ocean.

The first new style sheet will start out enabled, but the second
one will be disabled because /although/ selectedStylesheetSet's value
hasn't changed, lastStylesheetSet's value has.


So now let's suppose I'm doing something less absurd and I'm going
to let the alternate styles *behave* like alternate styles.

The page has a dynamic effect I want to highlight with CSS.
I put a set of rules in the *accent.css sheets; I can enable/disable
them with scripting.

   My script dynamically disables accent.css, blue-accent.css, and
   green-accent.css.

   The user chooses the Ocean style again.

   In response to some input, my script dynamically enables
   accent.css and green-accent.css (having detected that "Forest"
   is the chosen set).

   In response to some further input, my script dynamically disables
   accent.css and green-accent.css.

   The user decides to check out the Forest style and switches to it.

   All the Forest style sheets get enabled. Including green-accent.css.
   Which is wrong! What /happened/? Didn't touch anything, just went to
   change the style... The input-feedback association is broken.

   And accent.css stays disabled, too, so the page also /looks/ broken.

   So what do I have to do? Add/remove the DOM nodes representing the links?
   (Isn't that going to require reloading & reprocessing the style sheet?)

> a UA defined value representing a persisted stylesheet set selection
> for the document

How does this interact with page loading? (You can assume a StyleHistory
object with a string GetSelectionFor(DocumentStyle aDocStyle) or
void SetSelectionFor(DocumentStyle aDocStyle) method, if it helps.)

>> something that many pages on the Web already do
>
> Are these pages you're talking about trying to implement a style switcher
> or are they just setting up alternate styles code for fun?

Both (mainly the former).


>> Note that from the DOM's perspective, all views have the same
>> selectedStylesheetSet.
>
> If you don't mind, I could use an explanation for the non-DOM-initiate.
> *bows apologetically*

http://www.w3.org/TR/DOM-Level-2-Views/views.html


>> The intended use of lastStylesheetSet is to make it easy to tell
>> what set sheets have to belong to to be enabled if they are
>> dynamically inserted. See the end of the spec (search for
>> "lastStylesheetSet").
>
> It took me awhile to figure out what you were trying to do. It's
> because enabling a style sheet from a non-selected set makes
> selectedStyleSet == "", right? You need it to store the last
> selection because the current situation is ambiguous wrt which
> set is the selected style set.

No, it's because dealing with the ambiguous case without this cached value is 
very bad for performance. (If you look at one of the earlier versions of the 
spec you'll see I had it defined without this attribute, but bz told me he 
couldn't implement that without taking a huge perf hit.)


> [...] and do the following (assuming your model):
>    1. The user selects Ocean from the Use Style menu.

first two and last three are enabled, lastStylesheetSet = selectedStylesheetSet 
= "Ocean".


>   2. The page scripts all style sheets disabled except  forest.css
>                                                    and  blue.css
>   3. The page adds a style sheet with title Ocean.

The new sheet is enabled since that was the last selected set.


>   4. The page checks the selectedStyleSet: it's the empty string.

Correct.


>      (If the user opens the Use Style menu, nothing will be selected.)

Correct.


>   5. The page sets the selectedStyleSet to the empty string.

The first two sheets are enabled. lastSelectedSet is set to the empty string.


>      (If the user opens the Use Style menu, Basic Page Style will be
>       sepected.)

Interesting point. The style switcher needs to check, when selectedStylesheetSet 
is the empty string, whether or not the persistent sheets are enabled and 
nothing else, or if it is an ambiguous situation.

Note that IIRC the CSS and HTML specs don't actually have a concept of "Basic 
Page Style". It's either persistent + an alternate set, or no CSS at all. (I 
think.)


>   6. The page adds another style sheet with title Ocean.

It isn't enabled. (lastStylesheetSet is the empty string.)


> So now let's suppose I'm doing something less absurd and I'm going
> to let the alternate styles *behave* like alternate styles.
>
> The page has a dynamic effect I want to highlight with CSS.
> I put a set of rules in the *accent.css sheets; I can enable/disable
> them with scripting. [...]

Your script is semantically flawed. You should set, e.g., a class on the root 
element and let the selectors accent the relevant elements.


> So what do I have to do? Add/remove the DOM nodes representing the links?

If you insist that the semantic you are scripting is purely stylistic, and does 
not represent a whole separate alternate stylesheet set, then yes, you would 
change the <link> elements directly. (e.g. change rel="stylesheet alternate" to 
rel="script-disabled-stylesheet".)


> (Isn't that going to require reloading & reprocessing the style sheet?)

Stylesheets should be cached.


>> a UA defined value representing a persisted stylesheet set selection
>> for the document
>
> How does this interact with page loading?

This is poorly defined. I'll write a new version that clarifies this.
> Both (mainly the former).

Is your concern about breaking backwards-compatibility mainly motivated by the
former, or strongly also by the latter?

> http://www.w3.org/TR/DOM-Level-2-Views/views.html

Doesn't help.

If I load the same page in two different windows, are they going to be related
like that or is it just for, e.g. print preview?

> Note that IIRC the CSS and HTML specs don't actually have a concept of "Basic
> Page Style". It's either persistent + an alternate set, or no CSS at all. (I
> think.)

If I don't specify a preferred sheet, the persistent style sheets are still
applied. This situation is represented by the Basic Page Style option.

> whether or not the persistent sheets are enabled and nothing else, or if it
> is an ambiguous situation.

It can just check to see if lastStylesheetSet is the empty string or not.

> Stylesheets should be cached.

In the layout system (parsed) or just in the file cache?

> you would change the <link> elements directly. (e.g. change rel="stylesheet
> alternate" to rel="script-disabled-stylesheet".)

I would have thought the intended purpose of the disabled property was for this
kind of thing, not for manipulating the alternate style selection.
> If I load the same page in two different windows, are they going to be related
> like that or is it just for, e.g. print preview?

If you just load it, no.  It's not just for print preview, though print preview
is a good examplt; there are many different ways that multiple views of the same
document could be presented at once.  Consider editor, for example.  All three
of its views could really be considered different visual representations of the
same DOM....
> Is your concern about breaking backwards-compatibility mainly motivated by the
> former, or strongly also by the latter?

My main concern is with not breaking interoperability unnecessarily, especially 
when one of the primary implementations is not going to change for at least 3 
years, if ever. Compatibility with current scripts is less important than 
keeping future scripts compatible with existing UAs.


>> Note that IIRC the CSS and HTML specs don't actually have a concept of "Basic
>> Page Style". It's either persistent + an alternate set, or no CSS at all. (I
>> think.)
>
> If I don't specify a preferred sheet, the persistent style sheets are still
> applied. This situation is represented by the Basic Page Style option.

Fair enough.


>> whether or not the persistent sheets are enabled and nothing else, or if it
>> is an ambiguous situation.
>
> It can just check to see if lastStylesheetSet is the empty string or not.

True.


>> Stylesheets should be cached.
>
> In the layout system (parsed) or just in the file cache?

That is UA-dependent. I wouldn't be surprised if parsing the sheet took less 
time than applying it anyway (thus making the distinction unimportant).


> I would have thought the intended purpose of the disabled property was for 
> this kind of thing, not for manipulating the alternate style selection.

I doubt that whoever invented "disabled" (probably someone at Microsoft) had any 
idea about alternate stylesheets. However, the practical outcome has been that 
disabled is largely used to manipulate the altss selection.
>> whether or not the persistent sheets are enabled and nothing else, or if it
>> is an ambiguous situation.
>
> It can just check to see if lastStylesheetSet is the empty string or not.

Actually that's not quite true. Before it is selected the first time, but after 
the DOM has been played about with, selectedStylesheetSet will be blank along 
with lastStylesheetSet but the set that will be used to determine whether things 
are enabled or not is preferredStylesheetSet and it is possible to be in this 
state without having all persistent stylesheets disabled.
Summary: Implement scriptable method of choosing a stylesheet set off the document object → Implement scriptable method of choosing a stylesheet set off the document object (alternate altss)
Just an observation... Safari actually treats .disabled and alternate stylesheet selection as completely 
independent, and this massively simplifies this spec.  I only needed the two properties and am actually 
baffled/confused by how complicated the proposal has grown.  The complications arise entirely from 
the fact that you're overloading .disabled and alternate stylesheet selection.

In Safari there are just two properties: the selected set and the preferred set.  Sheets that are enabled/
disabled via script are completely independent, and a sheet enabled via script stays enabled if you set 
the selected set etc. via this API.

The more I see how complicated this has gotten, the more I am against implementing alternate 
stylesheets on top of the .disabled property.
hyatt, I'm a little confused about your impl... in each of the following four
states, is the sheet applied to the document?

1)  DOM property disabled is false, set sheet is in selected via this API
2)  DOM property disabled is true, set sheet is in selected via this API
3)  DOM property disabled is false, set sheet is in NOT selected via this API
4)  DOM property disabled is true, set sheet is in NOt selected via this API
.disabled essentially has three states in Safari.  Those values are "false" (the default value for all sheets, 
alternate or otherwise), "force false" and "force true".  The latter two are what you get when you enable/
disable the sheet via script.  .disabled trumps anything you do with alternate sheets and is applied on 
top of any sheet settings.

So assuming I understand the four states you're asking me about properly, and assuming the disabled 
values were set *by the Web page explicitly*:
(1) Yes
(2) No
(3) Yes
(4) No

Assuming that the disabled values of "false" were just the default (i.e., script didn't monkey with the 
property, then the answer to (3) changes to "No".
Notice how much simpler things become if you don't try to "build the stylesheet state" out of what you 
happen to have enabled/disabled.
Note that this also matches WinIE, in the sense that if you query an alternate stylesheet in WinIE that is 
NOT APPLIED, you will get an answer of "false" for its disabled property.  If you then set the disabled 
property to "false", then the sheet enables.  This is how Safari behaves as well.
Yes but WinIE and Safari have no alternate stylesheet UI.

Your system totally fails to update the UI when the script changes the 
stylesheet set, which for me is the entire point of this interface.
Not to mention that the concept of a "boolean"-type attribute having three 
states rings almost every programming design alarm bell I have.
Yeah, sure.  If we make disabled not be a boolean, all sorts of stuff can
happen.  The point is, it IS a boolean in Mozilla and I'd like to not change that.

Not to mention that:

  foo.disabled = foo.disabled

changing the page layout is just not cool, no matter whether IE does it.
I don't really like the odd way .disabled works here, but I still think it's more consistent to keep 
.disabled separate from the concept of sets.

In other words, I'd rather propose an addition of a new property to query for the tri-state .disabled and 
then have a simple set selection API then to build everything on top of .disabled and have a really 
complicated set selection API.
How would that affect the "foo.disabled = foo.disabled" issue?
Well, it wouldn't.  That part would suck, but you'd deprecate .disabled in favor of a new property.
> Compatibility with current scripts is less important than
> keeping future scripts compatible with existing UAs.

Future scripts will be written to be compatible with the
future UA set. What is the problem you have in mind, specifically? 

> However, the practical outcome has been that disabled is
> largely used to manipulate the altss selection.

Only because there's been no other way to do so.

> Before it is selected the first time, ...
> preferredStylesheetSet... without having all persistent
> stylesheets disabled.

What does disabling persistent stylesheets have to do with
anything?

Also, Basic Page Style only exists when there is no preferred
style set. If there's a preferred style set, then the option
isn't there.

> foo.disabled = foo.disabled
>
> changing the page layout is just not cool, no matter whether
> IE does it.

But
  ds.selectedStylesheetSet = ds.selectedStylesheetSet
changing the page layout is?

> Not to mention that the concept of a "boolean"-type attribute
> having three states rings almost every programming design
> alarm bell I have.

true, false, null. It's been done before.

> .disabled trumps anything you do with alternate sheets and
> is applied on top of any sheet settings.

IMO, the style set selection should trump anything you do with
.disabled. If you want to control a sheet with .disabled, just
make sure it's either persistent or part of the selected style
set.

BTW, what happens to the DOM if the same style sheet is linked
under different titles?
Blocks: altss
Summary: Implement scriptable method of choosing a stylesheet set off the document object (alternate altss) → [AltSS] Implement scriptable method of choosing a stylesheet set off the document object
> BTW, what happens to the DOM if the same style sheet is linked
> under different titles?

Then there are multiple sheet objects in the DOM.
> What is the problem you have in mind, specifically?

IE6 will be the majority Web browser until well after 2006.
> What does disabling persistent stylesheets have to do with
> anything?

I meant alternates, my bad. Anyway I've changed the spec now (and reved it) so
that this is no longer a problem.


> Also, Basic Page Style only exists when there is no preferred
> style set. If there's a preferred style set, then the option
> isn't there.

Oh. I didn't realise that. Ok. Spec takes that into account too now.


> But
>  ds.selectedStylesheetSet = ds.selectedStylesheetSet
> changing the page layout is?

I believe with the latest revision to this spec, this no longer ever happens.


> true, false, null. It's been done before.

So have memory leaks and null-pointer dereferences. That doesn't make them good.
(And true/false/null isn't a boolean, it's a pointer, reference, or variant.)


> IMO, the style set selection should trump anything you do with
> .disabled. If you want to control a sheet with .disabled, just
> make sure it's either persistent or part of the selected style
> set.

I understand what you want. I just don't think it is workable given what we have
(IMHO) to be compatible with. (See comment 5.)
fantasai, yes, I see what you mean regarding the style selection having to trump .disabled, since Web 
sites are going to set cookies and pick sheets via .disabled, so the style UI will need to be able to trump 
.disabled when the user picks a set.

So we could go in the other direction and have .selectedStylesheetSet only really do a selection 
(trumping out .disabled) when you set it via script.  We could say that this value is empty when no 
selection has been imposed by an external force (either UI or script).

So if I'm understanding this correctly, it would work like this... if the page sets the preferred set to A, 
but doesn't mess with any of the sheets, and you ask what the selectedSet is, you'd get "" and not A.  
Maybe the page then goes and disables a sheet or two from A and then enables a sheet or two from B.  
The selectedSet would still be "".  Then if you go and set the selectedSet to B, all the sheets in A get 
turned off, all the sheets in B get turned on, and now any .disabled setting that goes on would have no 
effect at all, since the user made a choice from the UI and that starts trumping anything the page might 
want to do?

That could be weird, since that could lead to the perception that "disabled" isn't working after you make 
a choice from the stylesheet UI, but maybe that's ok.
 
> That could be weird, since that could lead to the perception that "disabled" 
> isn't working after you make a choice from the stylesheet UI, but maybe that's 
> ok.

I don't think it is, especially given that it works today.
> > What is the problem you have in mind, specifically?
>
> IE6 will be the majority Web browser until well after 2006.

I meant what sort of scripts, not which UA.

> So have memory leaks and null-pointer dereferences.

I was thinking of SQL, actually.

> I understand what you want. I just don't think it is
> workable given what we have (IMHO) to be compatible with.
> (See comment 5.)

I don't understand what /you/ want, which is the problem.
What exactly are the backwards-compatibility issues?
In other words, what existing scripts would we break and
how important are they? I'm trying to figure out what's the
best thing to do, but I can't if I don't know what the
constraints are. I might come to the same conclusion you
did, but it's possible I'll come up with something else. :)

> since Web sites are going to set cookies and pick sheets
> via .disabled, so the style UI will need to be able to
> trump .disabled when the user picks a set.

That's not exactly what I meant, but it does bring up a good
point.

I'm thinking more like having the selected set be a sort of
"window". Only style sheets in that "window" can be applied
to the document. Whether the style sheet is enabled or
disabled is independent of this: it is only applied if it's
in the selected set /and/ enabled.

I don't see the documents style sheets as a list of style
sheets, with title and disabled attributes, that must somehow
be organized to handle altss. I see each style sheet as
belonging to a set: either the persistent set or an alternate
style set. A style sheet must be in an active set to have any
effect. The persistent set is always active. An alternate
set is only active if it's selected. The disabled attribute
is integral to the style *sheet* and doesn't affect the
status of the style *sets*.

The /problem/ with designing the interfaces around this
conception is the current mixed-up handling of altss out
there. What I /don't/ know, and Hixie seems to, is what
exactly the obstacles are. If the only problem is making
sure JS altss switchers work, then a hack might work:

  If all enabled style sheets belonging to an alternate
  set are enabled and *only* those style sheets are enabled,
  then we make that the selected set.
  (The state of persistent sheets is not taken into account.)

The JS style switcher would work, and our UI would reflect
the page's selection. However, trying to change the selection
with our UI wouldn't do anything because that conditional
would switch the selection back to the JS-selected set.

The website could let us handle switching by scripting to
use the selectedStylesheetSet interface in browsers that
support it. Something like:

  if (ds.selectedStylesheetSet) {
    ds.selectedStylesheetSet = "Title"
  }
  else {
    //loop over style sheets and set disabled
  }

I don't think our interface would be cooperating well with
a cookie-based one anyway, so disabling it for that kind
of website might not be such a bad idea...
>>> What is the problem you have in mind, specifically?
>> IE6 will be the majority Web browser until well after 2006.
> I meant what sort of scripts, not which UA.

Right, but I meant which UA. That's my point. Whatever we implement, IMHO, has
to be a superset of what all the other UAs are going to have for the next three
(or more) years, otherwise we are taking an interoperable status quo, and making
it _less_ interoperable. What you are proposing is not -- it doesn't matter what
the scripts do, if they have alternate stylesheets and use the disabled
attribute, then you are requiring that our (DOM) interfaces not act the same way
as other UAs.

The only things I'm happy to break compatibility over are those like the one
mentioned at the end of comment 5 where what current UAs do isn't interoperably
implemented, especially where it doesn't make any sense anyway.
 - Any script that works under this scheme will also work under other UAs.
  - unless it requires the new interfaces, in which case it wouldn't under your
    scheme, either
 - Any script that works in other UAs will also work in this one
  - unless it enables style sheets that aren't in an active set -- which
     a) is uncommon
     b) shouldn't be done anyway
     c) is problematic in your scheme, too, since any explicit style set
        switch by the user will mess up the author's scripted style sheet
        states (What's the point if it'll just get arbitrarily reset by a
        curious user?)

What do you think, hyatt?
> If all enabled style sheets belonging to an alternate
> set are enabled and *only* those style sheets are enabled,
> then we make that the selected set.
> (The state of persistent sheets is not taken into account.)

When? When the disabled attribute is toggled?
When the disabled attribute is toggled and when selectedStylesheetSet is changed,
yes.
Just as a clarification: the latter case is for locking the UI to the same state
as a site's .disabled-based altss switcher.
While I can see the attraction of such a system, it does seem rather peculiar
that the "disabled" attribute would have such a "stunted" effect. It also means
that you get a different effect if you disable sheets in the last set then
enable those in the new set, vs enabling then disabling (in the former case, you
get all the cases in between, whereas in the second case, you get the disabling
cases then all the new stylesheets at once).

It also seems weird that changing disabled can change the selectedStylesheetSet
but not vice versa. And in your version, a JS switcher will basically make the
UI useless (since if you do as you propose, the UI can't change the altss, and
if you ignore the bit about setting selectedStylesheetSet when
selectedStylesheetSet is changed, the stylesheets in the other sets will be
disabled), whereas with my system the two are designed to work together (which I
really considered a requirement).

I've tried to see if your proposal could work, or how to change it to make it
work, but I just don't see how, on balance, the improved purity of the proposal
really counter-balances the impedance mismatch between it and "disabled".
> And in your version, a JS switcher will basically make the UI useless [...]

To clarify: I think a specification that by design forces the user interface to
be useless is fundamentally flawed, and I couldn't in good faith recommend,
either to bz (who is volunteering to implement this here) or to the W3C (who I
hope will take the proposal that results from this discussion and standardise
it) that it be adopted and deployed.
> While I can see the attraction of such a system, it does seem rather
> peculiar that the "disabled" attribute would have such a "stunted" effect.

I don't see how subjecting the 'disabled' attribute's effect to
the style set switch is "stunting". It seems logical to me. *shrug*

> It also means that you get a different effect if you disable sheets...
> then enable those in the new set, vs enabling then disabling

I'd imagine it's usually done by looping straight through the
list of style sheets, enabling or disabling each based on its
'title' attribute. So that's not really a practical issue, right?

As for the peculiarness of the interaction between .disabled and
selectedStylesheetSet, I see it as the UA interfering to produce
it's best interpretation of what the author and the user intend,
not as an inherent property of the system -- which is why I
introduced it as a hack. If JS switchers weren't out there, this
wouldn't be necessary.

> I think a specification that by design forces the user interface to
> be useless is fundamentally flawed

And I think a specification that by its fundamental design creates an
ambiguous state, both internally and in the user interface, is also
flawed.

> I've tried... but I just don't see how...

Fair 'nuff. I'm just here to chatter, anyway; it's not my call.
> And I think a specification that by its fundamental design creates an
> ambiguous state, both internally and in the user interface, is also
> flawed.

Oh, I agree. But that is already inherent in the existing implementations of
"disabled", my proposal isn't making it worse as far as I can tell. :-)

fantasai: Let me know if you get any ideas on how we can solve this without any
of the problems found so far. I wish there was a better solution.

bz: I think the current proposal is reasonably stable.
   http://www.hixie.ch/specs/css/dom/altss
Well, haven't come up with another solution, but I've got another problem for you:

Suppose a site implements
  http://www.alistapart.com/articles/alternate/
,which uses cookies to store the style selection and onload to set the style
sheet states, with two alternate style sets: Ocean and Forest.

If I click the site's button for Forest,
then select Ocean from the Use Style menu,
what happens next time I visit the page (assuming UA style history)?
Do I see Ocean or Forest?

What if I first select Forest from Use Style,
then Ocean from the site's interface?
Assuming I understand the scripts they show correctly:

> If I click the site's button for Forest, then select Ocean from the Use Style 
> menu, what happens next time I visit the page (assuming UA style history)?

Ocean is selected. (Whether the UA implements style history or not is not 
important in this case because the onload event from the page would override the 
UA's peristence anyway.)


> What if I first select Forest from Use Style, then Ocean from the site's 
> interface?

Ocean is still selected.
> Ocean is selected. (Whether the UA implements ...

Wait. Say that again?
Ocean is selected. Why, does this surprise you? Like I keep saying, the whole
point of my proposal is that it integrates with the existing stylesheet switchers.
Ian, the statements "Ocean is selected" and "Whether the UA ..." are mutually
contradictory, since the site would set the set back to Forest, no?

And the problem, of course, is that the user would come back and not see the
site with the set it had when they left...  whereas if they do the same steps in
the opposite order "magically" it works.
> Ian, the statements "Ocean is selected" and "Whether the UA ..." are mutually
> contradictory, since the site would set the set back to Forest, no?

No. The script fantasai asked about uses an onunload handler and decides what
the selected sheet is by peeking at the disabled and titles attributes of the
linked sheets, not by remembering what the last user selection was.
Silly me. :) I should have linked to this page instead:
http://www.notestips.com/80256B3A007F2692/1/NAMO5GK2NM
There are three switchers on that page.

For the first one, the UA persistence would likely override the page's
persistence, since the page persistence code is run in an inline <script> block,
whereas I'm proposing the persistence code be run when the UA first needs to
determine what stylesheets to use. However, there is admittedly a race condition
here, and therefore the possibility that the script can override the
persistence. So the answer is probably Ocean, maybe Forest in rare cases.

For the second, there aren't any alternate stylesheets, so the point is moot
(you can't select the style from the UA UI).

The third is basically a combination of the first two, so the answer for that
one is "both of the above".
So is it better for UI to be effectively inconsistent or consistently ineffective?
Eh? It's better for a design to work in more cases, obviously.

I don't really understand what you mean. My spec works basically perfectly in
both of the examples you gave, with only two exceptions: a theoretical race
condition that I can't imagine would occur in the read world, and a case where
the page is semantically dubious (there aren't any alternate styles, the page is
effectively rewritten when the user selects a different set).

It's neither effectively inconsistent nor consistently ineffective -- it is
consistently effective. Or have I missed something?
> Or have I missed something?

Nah, I'm just having a hard time understanding how this interacts with style
history. Makes me wonder if maybe I'm getting duller as time goes on... anyway.

Are you saying that the selectedStylesheetSet attribute is calculated every time
a .disabled value is changed or just whenever someone looks up its value?
> Are you saying that the selectedStylesheetSet attribute is calculated every time
> a .disabled value is changed or just whenever someone looks up its value?

That's an implementation detail. (I believe bz wants to do it as the second one.)

Note that the spec does describe how persistence would work, in a (relatively
new) section at the bottom.
> That's an implementation detail. (I believe bz wants to do it as the second
> one.)

So, after having selected the Ocean style set, but before I go to another page,
I right-click on a link and select "Open in New Window". Ocean is an alternate
style set on that page.

Do you see why you must write the selection to Style History at the moment it
happens instead of waiting for unload?
> Do you see why you must write the selection to Style History at the moment it
> happens instead of waiting for unload?

Oh, sure. I'm not attached to the may persisting works. (I've made the spec
slightly vaguer about this.)
Could we actually decide something here instead of endlessly blathering?  ;)
As far as I know, I have addressed all concerns in the current version of the
proposed specification:

   http://www.hixie.ch/specs/css/dom/altss/altss

I just had a quick look and can't see anything wrong. (Once we implement
stylesheet persistence there will be some interesting issues w.r.t. script
blocking, stylesheet loading, and stylesheet set persistence, but they should
not be insurmountable.)
Hixie, that spec is only useful if we have buy-in from Opera and Safari on it. 
Do we?  hyatt, what are your plans wrt Safari?
Opera almost certainly will not be doing any DOM-related work for alternate
stylesheets any time soon.

Why does that spec require buy-in from other vendors? As far as I am aware, it
is a superset of the current standards, and is fully backwards compatible with
them. That certainly was the intention.
Well.. it uses some of the same property names as what Safari implements but has
different behavior, no?  That's not exactly a desirable situation....
Yup, that's what happens when non-standard extensions get implemented without
going through a consensus-building process. :-)

Hyatt's implementation, as described in comments above, IMHO makes no sense. It
has tri-state booleans and fails to address my primary requirement, namely that
the UI be able to track changes made by existing JS-based (i.e. .disabled-based)
stylesheet switchers.

If we want to be good net citizens, we could prefix our attributes and methods
with "moz". Then we have the DOM interface we need internally, without clashing
with other UAs.
Well, rather, that's what happens when someone writes a spec, doesn't make it
clear that it's a very early draft, and then radically changes it... ;)

I may indeed implement this stuff with moz prefixes for now, not that that will
make the whole thing useful to web authors....  hyatt, could you possibly move
your impl over to a safari prefix until we've agreed on how this should work?
Given that the spec hyatt implemented was an e-mail with a quick proposal to a
W3C-internal list, exactly how much clearer could I make it that it was an early
draft? ;-) (The current spec says it explicitly right at the start under "Status
of this Document", btw.)

I'm not sure that prefixes would make it any less useful for authors.
Depends on what we're trying to provide the authors with, no?  ;)  If it's a
cross-browser standard solution....

In any case, maybe I can get to this in 1.7.
Target Milestone: mozilla1.5beta → mozilla1.7beta
> If it's a cross-browser standard solution....

...then we need to go through the W3C and it won't be in CR before May at the
earliest. Working on that... ;-)
Summary: [AltSS] Implement scriptable method of choosing a stylesheet set off the document object → [AltSS] Implement scriptable method of choosing a stylesheet set off the document object (altss dom)
The patch to bug 32372 implements preferredStylesheetSet because it's needed to
filter the menu options appropriately.
Note that I have a pretty complete impl of one of the various proposals for this
bug in my tree and have for a while now.  If someone's interested, speak up and
I'll try to figure out which files to diff...
Maybe you could give me some direction on how to do what dbaron's asking me to
do for the preferredStylesheetSet bit in bug 32372?
FWIW, I will probably be integrating:
   http://www.hixie.ch/specs/css/dom/altss/altss
...into the Web Apps 1.0 spec.
Attached patch This should do what you want (obsolete) — Splinter Review
This patch adds an nsIDOM3DocumentStyle interface and implements it.  Feel free
to modify as needed.

Note that with this patch in place, the scriptable method is mostly done.  The
remaining work involves interaction with the CSSLoader and persistence...
Blocks: 83663
If I'm understanding the situation correctly, Ian's model has a "selected style
sheet == indeterminate" state mainly to handle enabling two style sheets with
different titles in things like this:

  <link rel="stylesheet" href="partA.css" title="Part A">
  <link rel="stylesheet" href="partB.css" title="Part B">
  <link rel="stylesheet" href="partC.css" title="Part C">

not in things like this:

  <link rel="stylesheet" href="partA.css" title="Part A">
  <link rel="alternate stylesheet" href="partB.css" title="Part B">
  <link rel="alternate stylesheet" href="partC.css" title="Part C">

(Ian, correct me if I'm wrong about this, but this is what I recall from what
you said over the summer about practical issues with style switching.)

Given that, the problem isn't that we need to let multiple alternate style
sheets be enabled at the same time, but that we need to let multiple "preferred"
style sheets be enabled at the same time.


From the HTML 4.01 Spec:
  http://www.w3.org/TR/html40/present/styles.html#h-14.3

  1a) 'Authors may specify a number of mutually exclusive style sheets called
       alternate style sheets.'
  1b) 'To specify an alternate style sheet, set the rel attribute to "alternate
       stylesheet" and name the style sheet with the title attribute.'

  2a) 'The author may specify that one of the alternates is a preferred style
       sheet. User agents should apply the author's preferred style sheet
       unless the user has selected a different alternate.'
  2b) 'To make a style sheet preferred, set the rel attribute to "stylesheet"
       and name the style sheet with the title attribute.'
  2c) 'If two or more LINK elements specify a preferred style sheet, the first
       one takes precedence.'

  3a) 'Authors may also specify persistent style sheets that user agents must
       apply in addition to any alternate style sheet.'
  3b) 'To make a style sheet persistent, set the rel attribute to "stylesheet"
       and don't set the title attribute.'

Two conclusions:
  1. The selection-is-indeterminate state in Ian's proposal is in violation of
     the HTML 4.01 specification by point 1a.
  2. The HTML 4.01 specification *does not say* what to do with a style sheet
     that is specified as preferred but is not actually the preferred style
     sheet.

By 2., given the first set of style links given above, the UA could reasonably
treat Part B and Part C
  - as alternate style sheets (which we do)
  - as persistent style sheets (which IEWin does)
  - as ignored completely..
There is no suggested behavior.
This is a loophole in the spec.


Adopting IE's behavior and making non-preferred preferred sheets behave as
persistent would solve the problem outlined above.
  a) Authors could enable style sheets with different titles at the  same time.
  b) We can implement alternate style switching as switching between mutually
     exclusive sets.
  c) It will be spec-compliant.



So, from IE's interpretation we get - 

  - A style sheet that is linked as a preferred style sheet but
    isn't actually the preferred style sheet is treated as a
    persistent style sheet.

And from the mutually-exclusive style sets constraint - 

  - There is always one (and only one) selected style set at
    any given point in time.
  - Persistent style sheets belong to all sets. All other style
    sheets belong to only one style set.
  - If a style sheet is not in the selected style set, it
    does not take effect. In other words, a style sheet only
    takes effect if a) it is applicable to the media
                    b) it is enabled
                    c) it belongs to the selected style set

------------------------------

Here's a proposal for switching styles. I'm not sure how well
this one would work in practice since I don't know how people
currently combine .selectedStylesheetSet with .disabled

  - Style sheets all start out with .disabled == false

  - Setting document.selectedStylesheetSet to a string changes
    the selected style set to that value. It does not affect the
    enabled/disabled status of any style sheets.

  - Calling document.enableStylesheetsForSet with a string argument
     - enables all non-persistent style sheets with that title
     - disables all non-persistent style sheets with a different title
     - sets document.selectedStylesheetSet to that title

  - Setting the disabled attribute of a style sheet could also
    affect document.selectedStylesheetSet. Specifically, if at
    any point the selected style set has no enabled style
    sheets and some other style set has enabled style sheets,
    then document.selectedStylesheetSet changes to the name of
    the first style sheet set with at least one enabled style
    sheet.
  
Here's another proposal, in case that last one is not workable.

  - Style sheets start out with their disabled states matching
    whether or not they belong to the preferred style set.

  - Setting document.selectedStylesheetSet to a string changes
    the selected style set to that value. It also
     - enables all non-persistent style sheets with that title
     - disables all non-persistent style sheets with a different title

  - Setting the disabled attribute of a style sheet could also
    affect document.selectedStylesheetSet. Specifically, if at
    any point the selected style set has no enabled style
    sheets and some other style set has enabled style sheets,
    then document.selectedStylesheetSet changes to the name of
    the first style sheet set with at least one enabled style
    sheet. (Style sheet disabled states are not toggled because
    of document.selectedStylesheetSet's value change.)

Hixie, you're the keeper of "how web pages screw around with DOM/HTML/CSS"
knowledge. What do you think? (I am not interested in hearing about "we must
have a superset of IEWin's functionality just because", only about "we must also
handle this specific case which is used commonly under X conditions and
assumptions".)
>  - as persistent style sheets (which IEWin does)

Actually, IE treats all preferred style sheets as persistent style sheets, since
it has no sheet switching mechanism.

I do agree that the letter of the spec does not spell out in the section on
preferred sheets what should happen to preferred sheets that are overridden by
an earlier preferred sheet... or for that matter overridden by an HTTP header. 
But it seems pretty clear to me that the spec intends that the "preferred sheet"
simply indicate which stylesheet set is applied by default (in the absense of
HTTP headers to that effect).  This is pretty clear, to me, from section 14.3.1
of the HTML specification [1] (which you quoted from!), paragraph 3 and 4.  In
particular, paragraph 4 says:

   When a user selects a named style, the user agent must apply all style sheets
   with that name. User agents must not apply alternate style sheets with a
   different style name. 

The proposal violates this "must not" as far as I can tell, since paragraph 3
clearly says that the preferred style sheet application is equivalent to the
user selectinga sheet.

I'm all for doing something sane, compatible, and useful to authors here. But
let's not pretend to be following the HTML spec when we're not.  If we (Safari,
Opera, Mozilla) break it for the greater good (i.e. all break it in the same
way), then it's not so bad.  If we all start breaking it in slightly different
ways (which is what I'm seeing happening right now), then I'm not so interested
in implementing any of it.
> The proposal violates this "must not" as far as I can tell

If I link to a style sheet using the "preferred style sheet" mechanism, but by
rule 2c that style sheet is /not/ the preferred style sheet, then what is that
style sheet? I say it's neither alternate nor preferred nor persistent; it's
undefined.

You seem to be saying that it counts as "preferred" in the first sentence of
paragraph 3 but not in the second sentence of paragraph 3, which is a wildly
inconsistent interpretation afaict.

  "The author may specify that one of the alternates is a preferred style sheet.
   User agents should apply the author's preferred style sheet unless the user
   has selected a different alternate."
>  "The author may specify that one of the alternates is a preferred style
> sheet.

Right.  There are several ways of specifying it, per spec:

1)  Have an HTTP header that says so (takes precedence)
2)  Have a <meta http-equiv> that says so (checked after real HTTP headers)
3)  Have a <link> with rel="stylesheet" and that title and have it be the first
    such sheet.

All the spec says is that the second and subsequent <link> with rel="stylesheet"
and a title do NOT specify the preferred style sheet _set_.  That's all.  The
problem is the spec refers to "preferred style sheet set" as "preferred
stylesheet".  Given that, the two sentences you qupte are consistent with each
other.

At least as far as I can tell this is the setup in the HTML spec.
Could someone succintly explain exactly what part of the alternate stylesheets
DOM proposal is being discussed here, and explain exactly what text would
replace it? Preferably in no more than 10 lines of text...
I think fantasai's proposal would have been reasonable 4 years ago, but I don't
think we should change what we've been pushing for years about the static case
to fix the dynamic case.  We should just try to fix the dynamic case within the
constraints of what we do now for the static case.

(I'm not particularly interested in this issue, and never have been, but
fantasai seems to want me to make a decision on this.)
Well, unless someone can give a coherent explanation as to why it is not a good
idea, I am still strongly in favour of using the following proposal, which has
been stable for nearly a year. See comment 74.

   http://www.hixie.ch/specs/css/dom/altss/altss
I want this out of my tree.  ;)

Still needs testing; if someone has tests for this, that would be much appreciated.  I'm going to try to fully port the seamonkey stylesheet switcher to this API, but I'll still need to test how that interacts with the DOM.
Oh, and I still need to decide which interface I'm gonna stick this stuff on, of course....  Not sure how OK changing nsIDOMNSDocumentStyle is.
Drive by comment: 

+  // DOM method, so handle BeginUpdate/EndUpdate
+  mozAutoDocUpdate(mDocument, UPDATE_STYLE, PR_TRUE);
+  nsresult rv = nsCSSStyleSheet::SetEnabled(!aDisabled);
+  return rv;

I think mozAutoDocUpdate needs a variable name or end update will be called before SetEnabled.
Doh.  Good catch.  Fixed.
we need to do something about that. that mistake happens way too often. I suggest:

#define AUTO_DOC_UPDATE(doc, type, notify) \
  mozAutoDocUpdate autoDocUpdater__(doc, type, notify);

(SUSPEND_PUMP_FOR_SCOPE() works similarly)
(probably without that semicolon)
Even better, borrow from what I recently did to AUTO_MARK_JSVAL and do:

#define MOZ_AUTO_DOC_UPDATE_PASTE2(tok,line) tok##line
#define MOZ_AUTO_DOC_UPDATE_PASTE(tok,line) \
  MOZ_AUTO_DOC_UPDATE_PASTE2(tok,line)
#define MOZ_AUTO_DOC_UPDATE(doc,type,notify) \
  mozAutoDocUpdate MOZ_AUTO_DOC_UPDATE_PASTE(_autoDocUpdater_, __LINE__) \
  (doc,type,notify)

and you can use the macro twice in the same scope.
(Then again, maybe you never need one for two different documents in the same scope, but you might...)
Depends on: 333148
Fixed bug 333148 on adding those macros.  Seems like a very good idea to me.
Attached patch Updated to comments (obsolete) — Splinter Review
This passes the tests Anne was kind enough to write and put up at http://dump.testsuite.org/2006/alternate-stylesheet-api/ (and which I will add to our regression tests)
Attachment #217257 - Attachment is obsolete: true
Attachment #217636 - Flags: superreview?(dbaron)
Attachment #217636 - Flags: review?(bugmail)
From a quick skim, GetPreferredStyleSheetSet seems wrong -- my understanding of Ian's "as eventually defined elsewhere in this specification" is that it matches the concept of preferred as sort-of-defined in http://www.w3.org/TR/1999/REC-html401-19991224/present/styles.html#h-14.3.2
i.e., if Default-Style isn't set, it uses the first title for which alternate isn't one of the rel keywords.
Right.  That's handled by CSSLoaderImpl::IsAlternate, no?  At that point if preferred-style is not set yet (though perhaps I do need to watch for it being explicitly set to the empty string?  I should probably leave in that XXX comment), we'll set the preferred style to the title of the sheet, but only if it doesn't have "alternate" in the rel.
bz asked for testcases <http://weblogs.mozillazine.org/bz/archives/010020.html>, so here's a testcase.  It's for very basic functionality (just 4.11, not 4.11.1, 4.11.2 or 4.11.3), but should be easily extendable.

So, is this what you're after (or the start thereof, anyway)?
Bother, I didn't see comment 87. Sorry for the bugspam.
Actually, the more tests the better.  ;)

Note that Ian is changing the spec so that titles are case-sensitive (per HTML4 and all implementations but Gecko); I'll post an updated patch with that change.
Attached patch Now case-sensitive (obsolete) — Splinter Review
This is now case-sensitive.  I haven't changed the IDL comments yet; waiting on the spec update.  Or I could just remove all the comments and point to the spec...
Attachment #217636 - Attachment is obsolete: true
Attachment #217979 - Flags: superreview?(dbaron)
Attachment #217979 - Flags: review?(bugmail)
Attachment #217636 - Flags: superreview?(dbaron)
Attachment #217636 - Flags: review?(bugmail)
Comment on attachment 217979 [details] [diff] [review]
Now case-sensitive

>Index: content/base/src/nsDocument.h
>+  nsRefPtr<nsDOMStyleSetList> mStyleSetList;

Please name this nsDOMStylesheetSetList and mStylesheetSetList to match the spec.

>Index: content/base/src/nsDocument.cpp


>+nsDOMStyleSetList::Item(PRUint32 aIndex, nsAString& aResult)
>+{
>+  nsStringArray styleSets;
>+  nsresult rv = GetSets(styleSets);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+  
>+  if (aIndex >= (PRUint32)styleSets.Count()) {
>+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
>+  }

This should return null, not throw, when aIndex is out-of-bounds


>-nsDocument::GetPreferredStylesheetSet(nsAString& aStyleTitle)
>+nsDocument::GetSelectedStylesheetSet(nsAString& aSheetSet)
...
>+    if (aSheetSet.IsEmpty()) {
>+      aSheetSet = title;
>+      continue;
>+    }
>+
>+    if (!title.IsEmpty() && !aSheetSet.Equals(title)) {

I would use |else| rather than |continue| here, but that's up to you.


>+nsDocument::EnableStylesheetsForSetInternal(const nsAString& aSheetSet,
>+                                            PRBool aUpdateCSSLoader)
>+{
>+  PRInt32 count = GetNumberOfStyleSheets();
>+  nsAutoString title;
>+  BeginUpdate(UPDATE_STYLE);
>+  for (PRInt32 index = 0; index < count; index++) {
>+    nsIStyleSheet* sheet = GetStyleSheetAt(index);
>+    NS_ASSERTION(sheet, "Null sheet in sheet list!");
>+    sheet->GetTitle(title);
>+    sheet->SetEnabled(title.IsEmpty() || title.Equals(aSheetSet));

This seems wrong. The spec specifically says that "Style sheets that do not have a title are never affected by this method". So I would wrap an if-check around SetEnabled.

r=me with that
Attachment #217979 - Flags: review?(bugmail) → review+
> This should return null, not throw, when aIndex is out-of-bounds

I'll fix our existing DOMStringList impl too. Separate patch, I guess -- bug 334884.

> This seems wrong.

Good catch!
Attachment #152965 - Attachment is obsolete: true
Attachment #217979 - Attachment is obsolete: true
Attachment #219227 - Flags: superreview?(dbaron)
Attachment #217979 - Flags: superreview?(dbaron)
Summary: [AltSS] Implement scriptable method of choosing a stylesheet set off the document object (altss dom) → [FIX][AltSS] Implement scriptable method of choosing a stylesheet set off the document object (altss dom)
Target Milestone: mozilla1.7beta → mozilla1.9alpha
Comment on attachment 219227 [details] [diff] [review]
Updated to comments

>Index: dom/public/idl/stylesheets/nsIDOMNSDocumentStyle.idl

>+ * NOTE: This interface represents the additions to nsIDOMNSDocumentStyle

additions to nsIDOMDocumentStyle

>+ * defined by <http://whatwg.org/specs/web-apps/current-work/#scs-alternate>.

The link now seems to be http://whatwg.org/specs/web-apps/current-work/#alternate-style-sheets .  What's Hixie doing breaking URIs on us?

>+   * author. It is determined from the order of style sheet declarations and
>+   * the Default-Style HTTP headers, as eventually defined elsewhere in this
>+   * specification. If there is no preferred style sheet set, this attribute

Not sure if you really want to say "in this specification" if it's not clear that you're quoting.


I still have to review the nsDocument.h and .cpp changes, which are the bulk of the patch, but the rest looks OK.  And I need to read the spec more carefully...
Comment on attachment 219227 [details] [diff] [review]
Updated to comments

>Index: content/base/src/nsDocument.h
>+  nsRefPtr<nsDOMStylesheetSetList> mStylesheetSetList;

I'd call this mStyleSheetSets, since it's for the attribute styleSheetSets.  (Note capital S in Sheet.)

>+  // Member to store out last-selected stylesheet set.
>+  nsString mLastStylesheetSet;

I'd capitalize the s in sheet to be consistent with the interface and with our other code.

>Index: content/base/src/nsDocument.cpp

>+class nsDOMStylesheetSetList : public nsIDOMDOMStringList

Likewise, I'd capitalize the s in the name of this type.

>+  NS_INTERFACE_MAP_ENTRY_CONTENT_CLASSINFO(DOMStringList)

Is it OK to use the same classinfo as a different type?

>+nsDOMStylesheetSetList::Contains(const nsAString& aString, PRBool *aResult)

>+  *aResult = styleSets.IndexOf(aString) > -1;

I'd actually use "!= -1".

>+nsresult
>+nsDOMStylesheetSetList::GetSets(nsStringArray& aStyleSets)
>+{
>+  if (!mDocument) {
>+    return NS_ERROR_NOT_AVAILABLE; // Or should this be NS_OK?

I think it should be, so that the list becomes empty once the document goes away.  http://www.w3.org/TR/DOM-Level-3-Core/core.html#DOMStringList says "no exceptions"...

>+    if (!title.IsEmpty() && aStyleSets.IndexOf(title) == kNotFound &&

I'd use explicit -1 rather than kNotFound, since the nsVoidArray code seems to do that.  But either way this should be consistent with your other call.

>+nsDocument::GetSelectedStylesheetSet(nsAString& aSheetSet)

>+  nsCOMPtr<nsIDOMStyleSheet> domSheet;

domSheet is probably better off inside the loop

>-NS_IMETHODIMP
>-nsDocument::GetPreferredStylesheetSet(nsAString& aStyleTitle)
>-{
>-  CSSLoader()->GetPreferredSheet(aStyleTitle);
>+NS_IMETHODIMP
>+nsDocument::GetPreferredStylesheetSet(nsAString& aSheetSet)
>+{
>+  GetHeaderData(nsHTMLAtoms::headerDefaultStyle, aSheetSet);

I'm not a big fan of storing the same piece of information in two different places.  (It looks like it really is the same thing -- if not, then there's probably a bug somewhere.)  Maybe file a followup bug?

>+nsDocument::EnableStylesheetsForSetInternal(const nsAString& aSheetSet,
>+                                            PRBool aUpdateCSSLoader)
>+{
>+  PRInt32 count = GetNumberOfStyleSheets();
>+  nsAutoString title;
>+  BeginUpdate(UPDATE_STYLE);

Seems odd to put the BeginUpdate two lines in rather than right at the start.

sr=dbaron with these comments and those in my previous comment.  Sorry for the really long delay here.
Attachment #219227 - Flags: superreview?(dbaron) → superreview+
(Oh, and I may not have mentioned a few places to convert Stylesheet to StyleSheet, but I'd do that throughout in the code that you're adding and the code that you're touching.)
And, actually, now that I look closely -- the IDL is wrong there too.  I suppose Hixie's changed the case in the spec since you've implemented it.  (And I agree that it should be a capital S in Sheet.)
Yeah, Hixie did change the case (rev 73	of Wep Apps 1.0, dated 2006-06-27 15:13:00, checkin commet is "Move some sections around (wip)").

I also agree that the new casing is better.
Flags: in-testsuite?
Whiteboard: Note: see comment 87 and comment 90 for tests
> Is it OK to use the same classinfo as a different type?

Yeah, as long as you implement all the same interfaces and want the same behavior  wrt to the various scriptable helpers (which we do want here).

> Maybe file a followup bug?

Bug 366708 filed.

This addresses all the comments.  I had to make two minor chrome changes due to the case change for preferredStyleSheetSet.

This passes all of Anne's tests except test 11 (which I think is wrong at the moment) and the test in comment 90 (if I fix the casing and the bug in the way it tests for property support).
Checked in.  I filed bug 366712 on being able to send headers in mochitest, at which point I should be able to check in the tests... I'll probably try to do some tests with <meta http-equiv> too.
Status: NEW → RESOLVED
Closed: 17 years ago
Depends on: 366712
Resolution: --- → FIXED
Does anything need to be done to the implementation of the stylesheet switcher in navigator.js, in particular are there shortcuts available that can be used to implement stylesheetInFrame, stylesheetSwitchFrame and/or stylesheetFillPopup?
Yeah; I totally forgot about that code in my tree.  ;)  Filed bug 366887 on that.
Blocks: 366887
Keywords: dev-doc-needed
I've made sure these are listed here:

https://developer.mozilla.org/en/DOM/document

Actual docs will be written tonight or tomorrow.
You need to log in before you can comment on or make changes to this bug.