Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] Reporter
	Description • 13 years ago

The style editor automatically tidies stylesheets. Unfortunately this means that when we go to a specific line (e.g. from the rule view) it may not be the line that the rule is on. We need a sourcemap in order to fix this.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 3 • 10 years ago

:harth, reminder to attach your WIP patch.

	Heather Arthur [:harth]
	Comment 4 • 10 years ago

Here's the WIP I have from a few months ago.

It's using rework's parser/prettifier (https://github.com/reworkcss/css) to automatically prettify files as they're loaded into the style editor, on the client side. The patch does *not* attempt to hook up the source maps yet.

The beginnings of that are in toolkit/devtools/server/actors/pretty-printer.js. A lot of that code is stolen from the debugger's script.js, with the intention of sharing pretty-printer.js with the debugger after getting it working with the style editor.

Though it's prettifying on the client right now, there was a reason I thought that it would have to be done on the server. I think it had to do with the fact that both the inspector and style editor have to know about the new code locations. Which is why I wanted to fix bug 871423 first so we could share the style sheet actors between the two.

Looking at how long prettifying and generating the source map can take, it seems like we might want to go the route of the debugger and Chrome, and wait to prettify until the user presses a button.

	Tom Tromey :tromey
	Comment 6 • 9 years ago

One simple thing to do here instead is just change _getSourceTextAndPrettify
to call _updateStyleSheet if the sheet changes.  The the inspector picks up
the new lines and everything seems to work.

	Tom Tromey :tromey
	Comment 7 • 9 years ago

This plan turns out to be too simple in the case where the user opens the inspector
and clicks on a rule's link.  In this situation the style editor will open to the
wrong line.

	Tom Tromey :tromey
	Comment 8 • 9 years ago

A couple more notes on this.

The current model seems a bit weird.

If the rule view does an edit, the result is re-prettified on the style editor side.
That is, from this point of view the edits are done on the original text and then
re-presented to the user in a prettified way.

On the other hand, if the user does an edit in the style editor, then that exact
text is parsed and now becomes the canonical source.


It's a bit stranger for users using source maps.
In this case edits are necessarily only done to the minified code.
So if you are looking at "original sources", you won't see any edits.
(I don't think we can truly handle this case but at least we can give
a visual warning that something weird is going on.)


I tend toward trying to salvage the plan in comment #6.
However this plan also makes the diffing feature more difficult, at least if your
source form is "ugly" according to our prettifier.

	Tom Tromey :tromey
	Comment 9 • 9 years ago

It's possible to salvage the comment #6 plan by noticing whether the style editor
is open before the user clicks on a link in the rule view.  If it is not open, then
the target location would be (optionally) translated to the new value at some point.

The downside of this is that it is ugly.  Maybe it wouldn't be terrible if I moved the
checking-and-rewriting logic into the style editor itself somewhere.


Another take on this problem is not to automatically prettify.  That is, provide a button
that the user must press.  This avoids the above problem entirely.  However, it comes with
its own issue, which is that after editing the text, the button can't be un-clicked --
there's no way to un-prettify the source.  This might matter for the "diffing" feature.

	Tom Tromey :tromey
	Comment 11 • 9 years ago

A further issue is that CSS warnings emitted by the platform will use the
"uglified" locations before the style editor is opened; and (assuming the
comment #6 plan) the prettified locations after the style editor is opened.

	Tom Tromey :tromey
	Comment 12 • 7 years ago

I'm no longer working on this.

I still think the best long-term result is to have the style editor copy the way the
debugger works; which now is to let users explicitly request pretty-printing, and to
keep both the original and generated sources available (not toggle as the style editor
does now).

I think the best route for this particular bug, in isolation, is to change
prettifyCSS:

https://dxr.mozilla.org/mozilla-central/rev/20d57b9c4183973af4af5e078dff2aec0b74f928/devtools/shared/inspector/css-logic.js#155

... to also construct a mapping between the generated and original location of
each token.  This requires some logic changes, because currently the token-copying
is batched, but that may be less convenient when tracking locations.

Then, this spot:

https://dxr.mozilla.org/mozilla-central/rev/20d57b9c4183973af4af5e078dff2aec0b74f928/devtools/client/styleeditor/StyleSheetEditor.jsm#275

can apply the new mappings using the |applySourceMap| method on the toolbox's sourceMapService.

	Pulsebot
	Comment 15 • 5 years ago

Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d8c3d43cd53
Map minified CSS to prettyfied CSS. r=gl