Bug 1415940 Part 5: Change InspectorUtils::GetRelativeRuleLine to not remap line numbers if it introduces underflow. r=bz
authorBrad Werth <bwerth@mozilla.com>
Wed, 17 Jan 2018 14:40:45 -0800
changeset 454372 1d6ebcc1be8f0ac1efc7240ca11f7e4a33bbad0c
parent 454371 544075fc480034ea90a404790688e3666a07181f
child 454373 fc525a00ff0cc6aebbd5c696a88974d6ef519019
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1415940
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1415940 Part 5: Change InspectorUtils::GetRelativeRuleLine to not remap line numbers if it introduces underflow. r=bz MozReview-Commit-ID: 8ZhzPWubBg7
devtools/client/inspector/rules/test/doc_cssom.html
layout/inspector/InspectorUtils.cpp
--- a/devtools/client/inspector/rules/test/doc_cssom.html
+++ b/devtools/client/inspector/rules/test/doc_cssom.html
@@ -4,16 +4,19 @@
 <head>
   <title>CSSOM test</title>
 
   <script>
     "use strict";
     window.onload = function () {
       let x = document.styleSheets[0];
       x.insertRule("div { color: seagreen; }", 1);
+
+      // Add a rule with a leading newline, to test that inspector can handle it.
+      x.insertRule("\n\ndiv { font-weight: bold; }", 1);
     };
   </script>
 
   <style>
     span { }
   </style>
 </head>
 <body>
--- a/layout/inspector/InspectorUtils.cpp
+++ b/layout/inspector/InspectorUtils.cpp
@@ -270,24 +270,53 @@ InspectorUtils::GetRuleColumn(GlobalObje
 {
   return aRule.GetColumnNumber();
 }
 
 /* static */ uint32_t
 InspectorUtils::GetRelativeRuleLine(GlobalObject& aGlobal, css::Rule& aRule)
 {
   uint32_t lineNumber = aRule.GetLineNumber();
+
+  // If aRule was parsed along with its stylesheet, then it will
+  // have an absolute lineNumber that we need to remap to its
+  // containing node. But if aRule was added via CSSOM after parsing,
+  // then it has a sort-of relative line number already:
+  // Gecko gives all rules a 0 lineNumber.
+  // Servo gives the first line of a rule a 0 lineNumber, and then
+  //   counts up from there.
+
+  // The Servo behavior is arguably more correct, but harder to
+  // interpret for purposes of deciding whether a lineNumber is
+  // relative or absolute.
+
+  // Since most of the time, inserted rules are single line and
+  // therefore have 0 lineNumbers in both Gecko and Servo, we use
+  // that to detect that a lineNumber is already relative.
+
+  // There is one ugly edge case that we avoid: if an inserted rule
+  // is multi-line, then Servo will give it 0+ lineNumbers. If we
+  // do relative number mapping on those line numbers, we could get
+  // negative underflow. So we check for underflow and instead report
+  // a 0 lineNumber.
   StyleSheet* sheet = aRule.GetStyleSheet();
   if (sheet && lineNumber != 0) {
     nsINode* owningNode = sheet->GetOwnerNode();
     if (owningNode) {
       nsCOMPtr<nsIStyleSheetLinkingElement> link =
         do_QueryInterface(owningNode);
       if (link) {
-        lineNumber -= link->GetLineNumber() - 1;
+        // Check for underflow, which is one indication that we're
+        // trying to remap an already relative lineNumber.
+        uint32_t linkLineIndex0 = link->GetLineNumber() - 1;
+        if (linkLineIndex0 > lineNumber ) {
+          lineNumber = 0;
+        } else {
+          lineNumber -= linkLineIndex0;
+        }
       }
     }
   }
 
   return lineNumber;
 }
 
 /* static */ CSSLexer*