1422360 - UA stylesheets should not show up in devtools, *including* when user is viewing about:preferences or about:addons

Status: NEW

(DevTools :: Style Editor, defect, P2)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Visit about:addons or about:preferences
 2. Hit F12 to open devtools.
 3. Click the "Style Editor" devtools tab, and scroll through the list of stylesheets.

ACTUAL RESULTS:
Our UA stylesheets are listed there as fully editable to the user -- for example, html.css, ua.css, minimal-xul.css.

EXPECTED RESULTS:
UA Stylesheets shouldn't be shown in devtools (at least, not unless the user has activated some "I am hacking on the browser" mode, e.g. perhaps if the user has ticked the checkbox to enable the browser chrome toolbox)

We depend on !important rules in these stylesheets to provide basic browser behavior, and we have C++ code that assumes that these rules have taken effect. So, if we expose an easy way for users to disable/mess with them, then it can lead to crashes or other unexpected behavior.

Presumably that's why these stylesheets aren't shown when you open devtools for a remote web page (like https://mozilla.org ).  However, right now they *are* shown for firefox-internal pages like about:addons and about:preferences.

Bug 1418349 is one example of badness that this can cause. (And even with the proposed crash-bandaid on that patch, users will still end up seeing something like this screenshot if we allow them to one-click-disable minimal-xul.css: attachment 8933114 [details])

Comment 1

[Patrick Brosset <:pbro>]

Thanks Daniel. Sounds like we should fix this sooner rather than later.
I think these should only be shown in the Browser Toolbox.

I think this is where we list the stylesheets today: https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/devtools/server/actors/stylesheets.js#733

We used to list stylesheets via document.styleSheets before, and in bug 1049199 we added this check and started using DOMUtils.getAllStyleSheets(document) for chrome documents. This was done so that we could edit stylesheets applied to XBL bindings. And this was made to work not only in the browser toolbox, but also in the normal toolbox, precisely for working on pages like about:preferences.

I'm not familiar enough with the different types of stylesheets this DOMUtils API returns, but it sounds like we should filter out the UA sheets in there.

Comment 2

[Patrick Brosset <:pbro>]

Brian, with your latest investigations around XBL, I think you might be able to help with this. Can you think of a way to filter out the UA sheets when we're not in browser toolbox mode?

Comment 3

[Brian Grinstead [:bgrins]]

We have a `devtools.inspector.showUserAgentStyles` pref that controls the visibility of these sheets in the inspector (which is false by default but true for the Browser Toolbox). We should:

1) Hide the UA sheets in the style editor behind the same pref
2) Given the issues / crashes that are possible here we should probably *also* disable editing of these files even if they are shown, like in BT or for a user who's flipped the pref (which is visible in normal devtools options)

Comment 4

[Brian Grinstead [:bgrins]]

For (1): rather than checking 'isChrome' we should check `Services.prefs.getBoolPref("devtools.inspector.showUserAgentStyles")`

Comment 5

[Brian Grinstead [:bgrins]]

For (2): looks like we have a 'readonly' prop that can be set on the editor: https://searchfox.org/mozilla-central/source/devtools/client/styleeditor/StyleSheetEditor.jsm#439. Off hand, I don't know what property to look at on the client to set it as readonly, but it'd be the same thing we use for the Inspector to make rules disabled and show "(user agent)" at the top of the rule.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset <:pbro> from comment #2)

Brian, with your latest investigations around XBL, I think you might be able
to help with this. Can you think of a way to filter out the UA sheets when
we're not in browser toolbox mode?

Comments 3, 4, and 5 answer this - clearing needinfo.