804120 - Offer a way to apply author stylesheet on a given document

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Matteo Ferretti [:zer0] [:matteo]]

With bug 737003 and bug 676054 fixed, this is a natural consequences.

We should be able to use the new method provided by bug 737003 with the new AUTHOR style type.
That will permit to us to register author stylesheet to specific document that won't result in the DOM of the page itself, and will give to us the capability to override the author stylesheet of the page, and solving all the issues that we currently have in Add-on SDK with `contentStyle`, described here:

https://github.com/mozilla/addon-sdk/wiki/contentStyle-issues

Notice that the AUTHOR sheet registered in this way should be applied always after all the AUTHOR sheet defined in the page (at loading time and created dynamically) in order to always override them.

Plus, if a global AUTHOR sheet is registered, the AUTHOR sheet per document should be able to override it.

Comment 1

[Gabor Krizsanits (INACTIVE)]

Attachment: Offer a way to apply author stylesheet on a given document

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #0)
> Plus, if a global AUTHOR sheet is registered, the AUTHOR sheet per document
> should be able to override it.

To meet this requirement, I really needed those tricks with INT32_MAX... so AddDocStyleSheet got bit more complex than it was... Shall I use some #defines for these special indexes? This is the reason for changing PresShell::AddAuthorSheet which belongs to the StyleSheetManager based AUTHOR_SHEETS which can now be overridden by document specific AUTHOR_SHEETS (added via DOMWinowUtils...).

I realized that it's easier to use the SheetTypeCount for defining the length of mAdditionalSheets with it than later assert it, if they are the same.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 675093 [details] [diff] [review]
Offer a way to apply author stylesheet on a given document

The only reason you need the extra complexity is because you're reusing AddDocStyleSheet in PresShell::AddAuthorSheet, right?

I think it would be better to just have that explicitly ask the document which sheet to insert before in the list (so just ask it for the pointer to the first additional stylesheet) and add an API on style set to insert a stylesheet before a given other stylesheet.

Comment 3

[Gabor Krizsanits (INACTIVE)]

(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 675093 [details] [diff] [review]
> I think it would be better to just have that explicitly ask the document
> which sheet to insert before in the list (so just ask it for the pointer to
> the first additional stylesheet) and add an API on style set to insert a
> stylesheet before a given other stylesheet.

I like that. Thanks for the idea! I'll file another patch with that change.

Comment 4

[Gabor Krizsanits (INACTIVE)]

Attachment: Offer a way to apply author stylesheet on a given document

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 675545 [details] [diff] [review]
Offer a way to apply author stylesheet on a given document

Do you still need the change to nsDocument::GetIndexOfStyleSheet?  Seems like it might be better to remove it and make the sheetDocIndex < 0 case in AddDocStyleSheet check whether "sheet" is the first additional sheet.  That would make the common case of no weird sheets fast...

>+  if (!mAdditionalSheets[eAuthorSheet].Count())
>+    return nullptr;
>+
>+  return mAdditionalSheets[eAuthorSheet][0];

  return mAdditionalSheets[eAuthorSheet].SafeObjectAt(0);

>+  // added with the StyleSheetManager

You mean StyleSheetService?

>+nsStyleSet::InsertBeforeStyleSheet(sheetType aType, nsIStyleSheet *aNewSheet,

How about InsertStyleSheetBefore?

You have a stray whitespace change in AddDocStyleSheet.

r=me with those fixed.

Comment 6

[Gabor Krizsanits (INACTIVE)]

Thanks for all the help.

https://tbpl.mozilla.org/?tree=Try&rev=432cbb0af161

https://hg.mozilla.org/integration/mozilla-inbound/rev/169e189085e2

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/169e189085e2