755606 - css rules tested in style editor don't apply the same way via page-mod

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Attachment: demo add-on

* use this site:
http://johnhammink.blogspot.ca/2012/05/part-3-updating-your-gingerbread.html

* use this css:
div.date-posts, div.post > h3.post-title {
    background-color: #EFEFEF;
    color: #232323;
}

div.date-posts { font-size: 115%; }

To repro:

1. open the style editor devtool and create a new stylesheet
2. paste the above css into it

Result: page now has a light bg in the post body, and dark title and body text

3. now instead inject the css into the page using page-mod & contentStyleFile, eg this add-on: 
https://github.com/canuckistani/simple-styler
4. install the add-on and open up the site

Result: the body text style is applied but the title style is not ( the title text remains loght-coloured! )

Expected result: these two methods of injecting css into a page should behave the same, eg the css should always be applied after all other style-sheets are loaded.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Matteo, can you look at this? If I remember correctly, the stylesheetservice take priority over everything else.

Comment 2

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

As Wes said, and as specified in the bug 634764, `contentStyle*` uses the stylesheet service to register the CSS as User Stylesheet. That means that they're always applied to the documents that matched the `include` pattern (using `-moz-document` in the CSS) not "before" or "after" the other stylesheets.

The specs are clear about the precedence:

"By default, rules in author style sheets have more weight than rules in user style sheets. Precedence is reversed, however, for "!important" rules. All user and author rules have more weight than rules in the UA's default style sheet."

See: http://www.w3.org/TR/CSS21/cascade.html

About the reason to have `contentStyle*` as User Stylesheet instead of inject them as <link> node into the code, see specifically this comment (the "regexp" part is not relevant for us, it was fixed): https://bugzilla.mozilla.org/show_bug.cgi?id=634764#c7

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

CCing Cedric to find out what the Style Editor does to apply CSS to the page.

Comment 4

[Jeff Griffiths (:canuckistani) (:⚡︎)]

also, I noticed I can work around this in some cases by making all of my injected css rules '!important'.

Comment 5

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

Jeff, that's exactly what I said in my previous comment. :)

https://bugzilla.mozilla.org/show_bug.cgi?id=755606#c2

It's how the User Stylesheet works.

Comment 6

[Dave Townsend [:mossop]]

Matteo, how does the style editor handle this and could we just do the same as them? Having them behave differently is a little confusing.

Comment 7

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

Dave, the Style Editor just add a <style> element at the end of the page: it doesn't use the stylesheet service so the stylesheet is not a User Stylesheet.
This approach was the first tried for `contentStyle` but wasn't considered good as you can see from the comments of the original bug for several reasons: https://bugzilla.mozilla.org/show_bug.cgi?id=634764

You can find a summary of them at comment 7:

https://bugzilla.mozilla.org/show_bug.cgi?id=634764#c7

And I agree with most of them. For a Style Editor, having this approach could be good enough because is a temporary change made at runtime. For add-on that consistently add this CSS rules I don't think is a good approach (they won't be protected from introspection and cascade won't be preserved for instance).

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Dave Townsend (:Mossop) from comment #6)
Dave, can you find someone on the platform team that could suggest an alternative to what we're doing that would be more in line with what Style Editor does?

Comment 9

[Dave Townsend [:mossop]]

dbaron might know if there is such a way, but the problem here is that we apparently decided we explicitly wanted to use user styles to be conformant with the CSS spec (see bug 634764 comment 7). Do we no longer want that?

Comment 10

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

As I pointed out, I personally think that the approach followed by Style Editor it's fine because is a temporary change.

If we use the same approach for page-mod, instead of register a User Stylesheet, I can see the following issues at the moment:

- Introspection:

1. Any website modified from the page-mod can detect that was modified just checking the additional <style> / <link>

2. If the add-on is popular, the website could tell if the user's browser has that add-on installed just checking the <style> / <link> added. For `contentStyleFile` it could be able to obtain the resource:// add-on path directly.

3. Any website modified from the page-mod is potentially able to revert all the changes just removing the <style> / <link> added by the page-mod.

- Cascade

1. Any element with inline stylesheet will overrides any CSS rules specified by the page-mod for the same element. If both of them have "!important", the inline stylesheet wins.

2. Any <style> / <link> added dynamically after the page-mod's stylesheet will overrides any CSS rules specified by the page-mod for the same element. If both of them have "!important", the stylesheet added dynamically wins.

(to sum up the two point above, there is no way for an add-on developer to ensure that the page-mod's style will be applied)

Comment 11

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #10)
> As I pointed out, I personally think that the approach followed by Style
> Editor it's fine because is a temporary change.
> 
> If we use the same approach for page-mod, instead of register a User
> Stylesheet, I can see the following issues at the moment:
> 
> - Introspection:
> 
> 1. Any website modified from the page-mod can detect that was modified 
> just checking the additional <style> / <link>

I don't care if about the website being able to detect modifications.

> 2. If the add-on is popular, the website could tell if the user's browser
> has that add-on installed just checking the <style> / <link> added. For
> `contentStyleFile` it could be able to obtain the resource:// add-on path
> directly.

I don't think this is a good way for an add-on to be detected by a site. I would prefer some channel being possible via web intents. If a website depends on an add-on, there should be tight and explicit coupling.

> 3. Any website modified from the page-mod is potentially able to revert all
> the changes just removing the <style> / <link> added by the page-mod.

I don't think this is what we want. If a user installs an add-on that alters websites, that's what they should get. 

> - Cascade
> 
> 1. Any element with inline stylesheet will overrides any CSS rules specified
> by the page-mod for the same element. If both of them have "!important", the
> inline stylesheet wins.
> 
> 2. Any <style> / <link> added dynamically after the page-mod's stylesheet
> will overrides any CSS rules specified by the page-mod for the same element.
> If both of them have "!important", the stylesheet added dynamically wins.
> 
> (to sum up the two point above, there is no way for an add-on developer to
> ensure that the page-mod's style will be applied)

This sucks - the add-on should be able to override the site's styling and impose its own. This to me is the essence of the feature. Additionally, I *really* like the development flow possibel by hacking out some css in the devtools style editor, then dropping the css into a jetpack to redistribute.

Comment 12

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

(In reply to Jeff Griffiths from comment #11)

> > If we use the same approach for page-mod, instead of register a User
> > Stylesheet, I can see the following issues at the moment:

> > - Introspection:

> > 1. Any website modified from the page-mod can detect that was modified 
> > just checking the additional <style> / <link>

> I don't care if about the website being able to detect modifications.

I'm just listing the potential issue I'm seeing if we're going to drop User Stylesheets in favor of adding a style at the end of the page, like the Style Editor does. This point basically opens the way to the points below.

> > 2. If the add-on is popular, the website could tell if the user's browser
> > has that add-on installed just checking the <style> / <link> added. For
> > `contentStyleFile` it could be able to obtain the resource:// add-on path
> > directly.
> 
> I don't think this is a good way for an add-on to be detected by a site.

I totally agree! In fact to me is a potential "security" issue, not a feature. An Add-on should be discoverable from websites only if it means to be. And definitely the website shouldn't be able to access to the add-on resource:// URL! If we adding a stylesheet as regular node in the DOM, that's what the website could obtain and use:

resource://jid1-swnnezz3sdbmxw-at-jetpack/mypage-mod/data/content.css

A workaround could be transform all CSS files in "data:" url. We already doing it because a constraint on the Stylesheet Service, but we're trying to remove it – there is a bug filled about it, by Alex, I can't find the id now.

But even doing it, we're not totally safe; as I said if the add-on is really popular the website could just compare the data: url in order to discovered it.

Maybe it's a small issue, but I honestly don't like that a website can see what are the add-on I'm using, it's a sort of "tracking me" isn't it? Unless, of course, the add-on was designed to be discoverable (and then the user should know about it).

> I would prefer some channel being possible via web intents. If a website
> depends on an add-on, there should be tight and explicit coupling.

Yeah, exactly. So, for instance, if I create a page-mod for gmail, Google shouldn't be able to trace which users are using my add-on, unless I explicitly want to make the add-on in that way (using a proper channel, definitely not CSS stuff).

> > 3. Any website modified from the page-mod is potentially able to revert all
> > the changes just removing the <style> / <link> added by the page-mod.

> I don't think this is what we want. If a user installs an add-on that alters
> websites, that's what they should get. 

Yes. That's because to me it's an issue. But if we're adding a node to the DOM, then the page can easily remove it.
If we register the `contentStyle` as User Stylesheet, the page can't do much because it's not discoverable, it's not part of the DOM. 

> > - Cascade

> > (to sum up the two point above, there is no way for an add-on developer to
> > ensure that the page-mod's style will be applied)

> This sucks - the add-on should be able to override the site's styling and
> impose its own.
> This to me is the essence of the feature.

That's because in the original they ended up to choose for User Stylesheet instead of adding a regular DOM Node. So we can take advantages of cascade.

In fact, if you open Style Editor, and you add a new stylesheet, and write a rule for an element that has an inline css rule with !important, you can't do much. From the Style Editor, you're not able to override it, even with "!important". 

The only way is inspect the element instead, and modify / remove the inline css rule.

> Additionally, I
> *really* like the development flow possibel by hacking out some css in the
> devtools style editor, then dropping the css into a jetpack to redistribute.

It could be a cool feature, but keep in mind that the nature of the Style Editor and a Page-Mod is different. The first one is made for temporary changes, in short "hacking". The second one is made for more "permanent" changes. So I can easily understand why they choose to add a <style> element at the end of the page, but that approach "as is" can't really works for us in my opinion, for the issues described above. And for just "hacking" those issues are not really a problem: even if you can't style something you can always inspect the element, change from inline style, or use the web console.

Maybe we need a third alternative, that both Add-on SDK and Style Editor will follows, so we can have this "export" feature. For instance, I'd like to have the `contentStyle` behaves like User Stylesheet with always `!important` rule, without the need to specify it all the times. At the moment I don't think we have an easy way to obtain it from the Add-on SDK.

Comment 13

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

I created a wiki page to put together all the `contentStyle` issues and see the whole picture:

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

This specific bug can be solved as soon as we have at least the platform feature described in bug 676054; so we're waiting for that.

Comment 14

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

This bug should be fixed now, by bug 837494. Feel free to re-open it if the behavior persist.