DevTools expects inIDOMUtils::GetRuleColumn to return a 1-based value, but Stylo returns a 0-based value. (The same is true for line numbers, but it appears Stylo lines are already 1-based.) The leads to failures in tests like: * devtools/client/inspector/computed/test/browser_computed_original-source-link.js which loads CSS file containing: --- div { color: #ff0066; } span { background-color: #EEE; } /*# sourceMappingURL=doc_sourcemaps.css.map */ --- The `div` rule is expected to have line 1 column 1, but Stylo gives line 1 column 0.

Example test log: https://treeherder.mozilla.org/logviewer.html#?job_id=113641285&repo=try&lineNumber=4276

:ferjm, maybe you are interested in this one?

This also appears to be the cause of ~8 failures in the tests devtools/client/inspector/boxmodel/test/browser_boxmodel*.

I keep finding more tests blocked on this one, so marking P1.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1) > Example test log: > > https://treeherder.mozilla.org/logviewer. > html#?job_id=113641285&repo=try&lineNumber=4276 I am afraid that I cannot reproduce this failure locally, but looking at the try log it seems that this is related to source maps. The errors says: "Column must be greater than or equal to 0, got -1" and comes from https://dxr.mozilla.org/mozilla-central/source/devtools/client/debugger/new/pretty-print-worker.js#5040 . Note that for lines it expects them to be greater or equal to 1. FWIW all the tests that I found with references to inIDOMUtils::GetRuleColumn/GetRuleLine are passing locally for me: - layout/inspector/tests/test_parseStyleSheet.html - layout/inspector/tests/test_bug856317.html - layout/inspector/tests/test_bug462789.html - layout/inspector/tests/test_bug462787.html - layout/inspector/tests/test_getRelativeRuleLine.html I need to investigate more.

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #5) > (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1) > > Example test log: > > > > https://treeherder.mozilla.org/logviewer. > > html#?job_id=113641285&repo=try&lineNumber=4276 > > I am afraid that I cannot reproduce this failure locally Actually, this is not true. I just realized that I was running the tests on a non-stylo build :\ Building and testing with stylo support now.

While the first test mentioned does use source maps, the others in comment 3 do not, so I don't believe it's related to source maps.

With the attached patch all the tests previously mentioned are passing except: - 2 tests from layout/inspector/tests/test_getRelativeRuleLine.html I filed bug 1382285 to fix them. - 2 tests from devtools/client/inspector/boxmodel/test/browser_boxmodel_pseudo-element.js which seem unrelated. I filed bug 1382288 for them.

Comment on attachment 8887990 [details] Bug 1380890 - stylo: make GetRuleColumn report a 1-based value. https://reviewboard.mozilla.org/r/158864/#review164272 ::: servo/ports/geckolib/glue.rs:1124 (Diff revision 1) > } > > match rules.0[index] { > CssRule::$name(ref rule) => { > let location = rule.read_with(&guard).source_location; > + // Gecko devtools expects 1-based line and column values. I am confused by this, because `SourceLocation` says[1] that its line and column indexes are 0-based, but clearly that's not what we're seeing. I'll redirect to Simon, who should have more context on this. [1]: http://searchfox.org/mozilla-central/source/third_party/rust/cssparser/src/tokenizer.rs#399

https://github.com/servo/rust-cssparser/pull/168 changed cssparser to use 1-based line and column numbers but did not update the doc-comment.

Sorry, I was confused. That PR changed *from* 1-based to 0-based. (And updated docs correctly.)

Comment on attachment 8887990 [details] Bug 1380890 - stylo: make GetRuleColumn report a 1-based value. https://reviewboard.mozilla.org/r/158864/#review165176 ::: servo/ports/geckolib/glue.rs:1127 (Diff revision 1) > CssRule::$name(ref rule) => { > let location = rule.read_with(&guard).source_location; > + // Gecko devtools expects 1-based line and column values. > + // source_location provides 1-based lines but 0-based columns. > *unsafe { line.as_mut().unwrap() } = location.line as u32; > - *unsafe { column.as_mut().unwrap() } = location.column as u32; > + *unsafe { column.as_mut().unwrap() } = location.column as u32 + 1; Instead of making this change here, let's add 1 to the column value in get_location_with_offset in rule_parser.rs. We can add the following comment there: // Column offsets are not yet supported, but Gecko devtools expect 1-based columns.

Comment on attachment 8887990 [details] Bug 1380890 - stylo: make GetRuleColumn report a 1-based value. https://reviewboard.mozilla.org/r/158864/#review165516

Fixed by https://github.com/servo/servo/commit/5cabb0f86bd71d86c5716f8504b8c46bfa1a7da8