1033082 - Media sidebar should display exact line numbers for @media rules

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Heather Arthur [:harth]]

Right now each @media rule link is to the line number of the first rule within the media ruleset. It should be the line of the @media rule itself.

Currently prevented by bug 591303.

Comment 1

[Heather Arthur [:harth]]

Attachment: Show exact line numbers for media queries

The platform bug was fixed, so we can use getRuleLineFor with @media rules now, this patch uses that.

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8457833 [details] [diff] [review]
Show exact line numbers for media queries

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

The code changes look fine.

I'm still seeing some weirdness on the line numbers on inline sheets but the weird thing is that it starts working once I add a new line in the style editor.  Even with this, it is an improvement over the current situation, so I'm fine with dealing with this in a follow up if you'd like:

* Open http://bgrins.github.io/devtools-demos/styleeditor/media.html
* Open style editor to second sheet.  Notice that the line #s are off a bit (4, 10, 25)
* Add a new line somewhere in the sheet.  Notice that the line #s snap to correct places.
* Open style editor to third sheet (which has the same contents, but not inline).  Notice the line #s (2, 8, 23)

May or may not be related, but I get a promise rejection after switching back and forth from the middle stylesheet: "Error: Unknown sheet source" (looks like that may be originating in sheetToUrl for csscoverage.js).

Comment 3

[Heather Arthur [:harth]]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> * Open style editor to second sheet.  Notice that the line #s are off a bit
> (4, 10, 25)

Yeah, getRuleLineFor reports line number from the top of the document ): There's a hacky function to account for this that we should use here.

> * Add a new line somewhere in the sheet.  Notice that the line #s snap to
> correct places.

> May or may not be related, but I get a promise rejection after switching
> back and forth from the middle stylesheet: "Error: Unknown sheet source"
> (looks like that may be originating in sheetToUrl for csscoverage.js).

Yeah, doesn't sound related to this change.

Comment 4

[Heather Arthur [:harth]]

Try:

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3a394accaab0