950921 - UI for flipping CSS source map pref

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Heather Arthur [:harth]]

Right now CSS source map support is hidden behind a pref. We should add UI for flipping the pref.

Comment 1

[Joe Walker [:jwalker]]

Is there an occasion when having provided a sourcemap, we'd like not to use it?

The best that I can think of is:
- The provided sourcemaps are broken (does this happen in RL? and what do we do if the original css 404s?)
- You'd like to see what was actually sent and can't use wget
- You might want to hide from some website that you were debugging their stuff (although if they provided sourcemaps, why would you want to hide? maybe only if you knew that their sourcemaps where only there to detect people debugging)

I'm not sure if these constitute a reason to turn sourcemaps off, but I feel like I'm missing something.

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Joe Walker [:jwalker] from comment #1)
> - The provided sourcemaps are broken (does this happen in RL? and what do we
> do if the original css 404s?)

We actually had to fix the debugger's handling of bogus sourcemaps in bug 906889.

The user needs to be able to toggle sourcemaps in order to inspect both the original and the modified source (SASS and CSS in this case), perhaps because he is not familiar with SASS, like me.

Comment 3

[Joe Walker [:jwalker]]

(In reply to Panos Astithas [:past] from comment #2)
> The user needs to be able to toggle sourcemaps in order to inspect both the
> original and the modified source (SASS and CSS in this case), perhaps
> because he is not familiar with SASS, like me.

I think it's possible that even for someone that doesn't understand SASS, the CSS is harder to read, but I can see that someone might want to check or compare the 2 sources.

Maybe a global pref isn't the right solution, and we're better off offering a "Show me the original" button. With growing support for CSS sourcemaps, this feels like a more generic solution, that doesn't just target people unfamiliar with SASS.

Comment 4

[Heather Arthur [:harth]]

Yeah, we need a pref for those who don't use Sass or LESS, and don't want to debug with it. If you're not looking at your own site, I bet you could want this regardless.

I think it makes the most sense as a global pref, because the inspector needs to know whether to show links to the originals, and this has to be coordinated with what we show in the Style Editor. It would help if we made this really easy to toggle back and forth.

We were talking about showing both the original and generated in the itchpad, one under the other. I don't know if I want to spend time dealing with the complications of that here.

For this, I'm thinking about making the pref apply immediately (it requires a reload right now), having a checkbox in the settings panel. And maybe doing a little toggle button or right click thing beside the filenames in the inspector and style editor.

Comment 5

[Heather Arthur [:harth]]

Attachment: WIP - options panel and rule view context menu

Work in progress. Adds an option to the devtools options panel under "Style Editor", and adds a "Show original sources" context menu item to the rule view. The preference change now applies immediately across the inspector and style editor.

The "checked" attribute of the rule view menu item isn't displaying, which I think is a XUL bug I've run into before.

Comment 6

[Heather Arthur [:harth]]

Attachment: Add checkbox to options panel and context menu item to rule view, computed view, and style editor

This patch adds a "Show original sources" checkbox to the options panel.

It also adds a menu item to the context menu when you right click in the rule view, computed view, or style editor. This allows easy toggling between showing original and plain CSS sources.

Comment 7

[Dave Camp (:dcamp)]

Comment on attachment 8357538 [details] [diff] [review]
Add checkbox to options panel and context menu item to rule view, computed view, and style editor

Review of attachment 8357538 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/styleeditor/StyleEditorUI.jsm
@@ +122,5 @@
> +
> +    this._contextMenu = this._panelDoc.getElementById("sidebar-context");
> +    this._contextMenu.addEventListener("popupshowing",
> +                                       this._updateSourcesLabel);
> +    

Trailing whitespace here.

Comment 8

[Heather Arthur [:harth]]

Attachment: Add checkbox to options panel and context menu to style panel and style editor

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a7073df6fe23

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/a7073df6fe23

Comment 11

[Francesco Lodolo [:flod]]

ruleView.contextmenu.showCSSSources.accessKey=C

Any chance of clashing with this?
ruleView.contextmenu.copy.accessKey=C

Comment 12

[Stefan Plewako [:stef]]

> +# LOCALIZATION NOTE  (saveStyleSheet.title): This is the file picker title,
> +# when you save a style sheet from the Style Editor.
> +saveStyleSheet.title=Save style sheet

saveStyleSheet.title and comment was added twice

Comment 13

[Francesco Lodolo [:flod]]

(In reply to Stefan Plewako [:stef] from comment #12)
> > +# LOCALIZATION NOTE  (saveStyleSheet.title): This is the file picker title,
> > +# when you save a style sheet from the Style Editor.
> > +saveStyleSheet.title=Save style sheet
> 
> saveStyleSheet.title and comment was added twice

3 times actually, and that's bug 960095