Closed
Bug 1398869
Opened 7 years ago
Closed 7 years ago
stylo: off-by-1 css rule line number when using stylo
Categories
(DevTools :: Inspector, defect, P2)
DevTools
Inspector
Tracking
(firefox57 fixed)
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: tromey, Assigned: tromey)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
If I apply the patch from bug 1388497 and then visit https://tromey.github.io/source-map-examples/css-warning/index.html, the resulting .scss line numbers are off by one in the console. servo is using 0-based lines here but gecko seems to use 1-based.
Updated•7 years ago
|
Summary: off-by-1 css rule line number when using stylo → stylo: off-by-1 css rule line number when using stylo
Comment 1•7 years ago
|
||
I suspect that https://github.com/servo/rust-cssparser/commit/4d42b6a68e5a4c8806fcbf7e67ec74187a9b8001 is involved, since we've dealt with this problem in the past before.
Assignee | ||
Comment 2•7 years ago
|
||
There are other options too unfortunately, like 0- -vs- 1- base differences for css and js in platform, or bad source maps, etc.
Assignee | ||
Comment 3•7 years ago
|
||
Logging shows maybe a fishy source map, since the original requests look reasonably sane. Using gecko: ################ MAP {"url":"https://tromey.github.io/source-map-examples/css-warning/ta.css","line":1,"column":218,"promise":{},"callbacks":[null]} => https://tromey.github.io/source-map-examples/css-warning/ta.scss 4 10 ################ MAP {"url":"https://tromey.github.io/source-map-examples/css-warning/ta.css","line":1,"column":237,"promise":{},"callbacks":[null]} => https://tromey.github.io/source-map-examples/css-warning/ta.scss 5 9 Using stylo: ################ MAP {"url":"https://tromey.github.io/source-map-examples/css-warning/ta.css","line":1,"column":212,"promise":{},"callbacks":[null]} => https://tromey.github.io/source-map-examples/css-warning/ta.scss 3 17 ################ MAP {"url":"https://tromey.github.io/source-map-examples/css-warning/ta.css","line":1,"column":231,"promise":{},"callbacks":[null]} => https://tromey.github.io/source-map-examples/css-warning/ta.scss 4 23
Assignee | ||
Comment 4•7 years ago
|
||
It's ok for the original column numbers to differ there, because it's up to the parser to choose the location at which to report an error. servo's choice seems a bit better in that it points to the property name, whereas gecko seems to point at the end of the name or maybe the ":".
Assignee | ||
Comment 5•7 years ago
|
||
My source map dumper uses 0-based lines and columns and says: column 211 source #0 orig line 2 orig column 17 column 230 source #0 orig line 3 orig column 23 So maybe nothing is incorrect here. Though I am wondering why some code in servo calls get_location_with_offset while other code does not.
Assignee | ||
Comment 6•7 years ago
|
||
Ok, that is the problem - there are paths that don't do this offsetting. Going to move this to a servo PR.
Assignee | ||
Comment 7•7 years ago
|
||
https://github.com/servo/rust-cssparser/pull/196
Assignee | ||
Comment 8•7 years ago
|
||
https://github.com/servo/servo/pull/18449
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 10•7 years ago
|
||
Of course there was fallout.
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8907276 [details] Bug 1398869 - regenerate console css message fixture; https://reviewboard.mozilla.org/r/178954/#review184032
Attachment #8907276 -
Flags: review?(wkocher) → review+
Comment 13•7 years ago
|
||
Pushed by kwierso@gmail.com: https://hg.mozilla.org/integration/autoland/rev/04720d1e6c02 regenerate console css message fixture; r=KWierso CLOSED TREE
Comment 14•7 years ago
|
||
Pushed by kwierso@gmail.com: https://hg.mozilla.org/integration/autoland/rev/23bf7009f21d Fix up test_bug413958.html, too a=bustage CLOSED TREE
Comment 15•7 years ago
|
||
Pushed by kwierso@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f4b0f3105c72 Now re-fix the non-stylo tests a=bustage CLOSED TREE
Comment 16•7 years ago
|
||
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
(In reply to Mozilla Autoland from comment #16) I manually landed it earlier to work around autoland being closed. This is just the original autoland request failing because it already landed.
Comment 18•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/04720d1e6c02 https://hg.mozilla.org/mozilla-central/rev/23bf7009f21d https://hg.mozilla.org/mozilla-central/rev/f4b0f3105c72
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•