Bug 1415940 Part 5: Change InspectorUtils::GetRelativeRuleLine to only not remap line numbers if it introduces underflow. draft
authorBrad Werth <bwerth@mozilla.com>
Wed, 17 Jan 2018 14:40:45 -0800
changeset 721877 c81f439f1adb43621ea77e3fc90fbda5573fb7d5
parent 721876 907b05d9ff666be7956aa7546a2b3f053a076155
child 721878 b512865ac99916f48dca42916211da1f9809c722
push id95979
push userbwerth@mozilla.com
push dateThu, 18 Jan 2018 00:40:34 +0000
bugs1415940
milestone59.0a1
Bug 1415940 Part 5: Change InspectorUtils::GetRelativeRuleLine to only not remap line numbers if it introduces underflow. 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 in the Servo case 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 in the Servo case, which is one
+        // indication that we're trying to remap an already relative
+        // lineNumber.
+        if (sheet->IsServo() && link->GetLineNumber() >= lineNumber) {
+          lineNumber = 0;
+        } else {
+          lineNumber -= link->GetLineNumber() - 1;
+        }
       }
     }
   }
 
   return lineNumber;
 }
 
 /* static */ CSSLexer*