Bug 1882964 - [devtools] Turn getRuleBodyTextOffsets into getRuleBodyText for easier unicode chars handling. r=layout-reviewers,devtools-reviewers,emilio,ochameau.

`InspectorUtils.getRuleBodyTextOffset` was returning bytes position, and we
were using them directly in Javascript `substring`, which causes problem
with non-ascii chars.
Instead of returning offsets to compute the rule string, we directly return
the string from InspectorUtils which is easier to work with.

Differential Revision: https://phabricator.services.mozilla.com/D204523

devtools/server/actors/style-rule.js

@@ -724,7 +724,7 @@ class StyleRuleActor extends Actor {
       const cssText = await this.pageStyle.styleSheetsManager.getText(
         resourceId
       );
-      const { text } = getRuleText(cssText, this.line, this.column);
+      const text = getRuleText(cssText, this.line, this.column);
       // Cache the result on the rule actor to avoid parsing again next time
       this._failedToGetRuleText = false;
       this.authoredText = text;

devtools/server/actors/utils/style-utils.js

@@ -4,8 +4,6 @@
 
 "use strict";
 
-const { getCSSLexer } = require("resource://devtools/shared/css/lexer.js");
-
 const XHTML_NS = "http://www.w3.org/1999/xhtml";
 const FONT_PREVIEW_TEXT = "Abc";
 const FONT_PREVIEW_FONT_SIZE = 40;
@@ -120,66 +118,12 @@ function getRuleText(initialText, line, column) {
     throw new Error("Location information is missing");
   }
 
-  const { offset: textOffset, text } = getTextAtLineColumn(
-    initialText,
-    line,
-    column
-  );
-  const lexer = getCSSLexer(text);
-
-  // Search forward for the opening brace.
-  while (true) {
-    const token = lexer.nextToken();
-    if (!token) {
-      throw new Error("couldn't find start of the rule");
-    }
-    if (token.tokenType === "symbol" && token.text === "{") {
-      break;
-    }
+  const { text } = getTextAtLineColumn(initialText, line, column);
+  const res = InspectorUtils.getRuleBodyText(text);
+  if (res === null || typeof res === "undefined") {
+    throw new Error("Couldn't find rule");
   }
-
-  // Now collect text until we see the matching close brace.
-  let braceDepth = 1;
-  let startOffset, endOffset;
-  while (true) {
-    const token = lexer.nextToken();
-    if (!token) {
-      break;
-    }
-    if (startOffset === undefined) {
-      startOffset = token.startOffset;
-    }
-    if (token.tokenType === "symbol") {
-      if (token.text === "{") {
-        ++braceDepth;
-      } else if (token.text === "}") {
-        --braceDepth;
-        if (braceDepth == 0) {
-          break;
-        }
-      }
-    }
-    endOffset = token.endOffset;
-  }
-
-  // If the rule was of the form "selector {" with no closing brace
-  // and no properties, just return an empty string.
-  if (startOffset === undefined) {
-    return { offset: 0, text: "" };
-  }
-  // If the input didn't have any tokens between the braces (e.g.,
-  // "div {}"), then the endOffset won't have been set yet; so account
-  // for that here.
-  if (endOffset === undefined) {
-    endOffset = startOffset;
-  }
-
-  // Note that this approach will preserve comments, despite the fact
-  // that cssTokenizer skips them.
-  return {
-    offset: textOffset + startOffset,
-    text: text.substring(startOffset, endOffset),
-  };
+  return res;
 }
 
 exports.getRuleText = getRuleText;

devtools/server/tests/xpcshell/test_getRuleText.js

@@ -39,7 +39,7 @@ const TEST_DATA = [
     input: "#id{color:red;background:yellow;}",
     line: 1,
     column: 1,
-    expected: { offset: 4, text: "color:red;background:yellow;" },
+    expected: "color:red;background:yellow;",
   },
   {
     desc: "Multiple rules test case",
@@ -48,14 +48,14 @@ const TEST_DATA = [
       "{ position:absolute; line-height: 45px}",
     line: 1,
     column: 34,
-    expected: { offset: 56, text: " position:absolute; line-height: 45px" },
+    expected: " position:absolute; line-height: 45px",
   },
   {
     desc: "Unclosed rule",
     input: "#id{color:red;background:yellow;",
     line: 1,
     column: 1,
-    expected: { offset: 4, text: "color:red;background:yellow;" },
+    expected: "color:red;background:yellow;",
   },
   {
     desc: "Multi-lines CSS",
@@ -72,7 +72,7 @@ const TEST_DATA = [
     ].join("\n"),
     line: 7,
     column: 1,
-    expected: { offset: 116, text: "\n  color: purple;\n" },
+    expected: "\n  color: purple;\n",
   },
   {
     desc: "Multi-lines CSS and multi-line rule",
@@ -98,75 +98,64 @@ const TEST_DATA = [
     ].join("\n"),
     line: 5,
     column: 1,
-    expected: {
-      offset: 30,
-      text: "\n    margin: 0;\n    padding: 15px 15px 2px 15px;\n    color: red;\n",
-    },
+    expected:
+      "\n    margin: 0;\n    padding: 15px 15px 2px 15px;\n    color: red;\n",
   },
   {
     desc: "Content string containing a } character",
     input: "   #id{border:1px solid red;content: '}';color:red;}",
     line: 1,
     column: 4,
-    expected: {
-      offset: 7,
-      text: "border:1px solid red;content: '}';color:red;",
-    },
+    expected: "border:1px solid red;content: '}';color:red;",
   },
   {
     desc: "Attribute selector containing a { character",
     input: `div[data-x="{"]{color: gold}`,
     line: 1,
     column: 1,
-    expected: {
-      offset: 16,
-      text: "color: gold",
-    },
+    expected: "color: gold",
   },
   {
     desc: "Rule contains no tokens",
     input: "div{}",
     line: 1,
     column: 1,
-    expected: { offset: 4, text: "" },
+    expected: "",
   },
   {
     desc: "Rule contains invalid declaration",
     input: `#id{color;}`,
     line: 1,
     column: 1,
-    expected: { offset: 4, text: "color;" },
+    expected: "color;",
   },
   {
     desc: "Rule contains invalid declaration",
     input: `#id{-}`,
     line: 1,
     column: 1,
-    expected: { offset: 4, text: "-" },
+    expected: "-",
   },
   {
     desc: "Rule contains nested rule",
     input: `#id{background: gold; .nested{color:blue;} color: tomato;  }`,
     line: 1,
     column: 1,
-    expected: {
-      offset: 4,
-      text: "background: gold; .nested{color:blue;} color: tomato;  ",
-    },
+    expected: "background: gold; .nested{color:blue;} color: tomato;  ",
   },
   {
     desc: "Rule contains nested rule with invalid declaration",
     input: `#id{.nested{color;}}`,
     line: 1,
     column: 1,
-    expected: { offset: 4, text: ".nested{color;}" },
+    expected: ".nested{color;}",
   },
   {
     desc: "Rule contains unicode chars",
     input: `#id /*🙃*/ {content: "☃️";}`,
     line: 1,
     column: 1,
-    expected: { offset: 12, text: `content: "☃️";` },
+    expected: `content: "☃️";`,
   },
 ];
 
@@ -192,7 +181,7 @@ function run_test() {
       }
     }
     if (output) {
-      deepEqual(output, test.expected);
+      Assert.equal(output, test.expected);
     }
   }
 }

dom/chrome-webidl/InspectorUtils.webidl

@@ -89,16 +89,16 @@ namespace InspectorUtils {
   sequence<DOMString> getRegisteredCssHighlights(Document document, optional boolean activeOnly = false);
   sequence<InspectorCSSPropertyDefinition> getCSSRegisteredProperties(Document document);
 
-  // Get the start and end offsets of the first rule body within initialText
+  // Get the first rule body text within initialText
   // Consider the following example:
   // p {
   //  line-height: 2em;
   //  color: blue;
   // }
-  // Calling the function with the whole text above would return offsets we can use to
-  // get "line-height: 2em; color: blue;"
+  // Calling the function with the whole text above would return:
+  // "line-height: 2em; color: blue;"
   // Returns null when opening curly bracket wasn't found in initialText
-  InspectorGetRuleBodyTextResult? getRuleBodyTextOffsets(UTF8String initialText);
+  UTF8String? getRuleBodyText(UTF8String initialText);
 
   // Returns string where the rule body text at passed line and column in styleSheetText
   // is replaced by newBodyText.
@@ -185,11 +185,6 @@ dictionary InspectorCSSPropertyDefinition {
   required boolean fromJS;
 };
 
-dictionary InspectorGetRuleBodyTextResult {
-  required double startOffset;
-  required double endOffset;
-};
-
 dictionary InspectorStyleSheetRuleCountAndAtRulesResult {
   required sequence<CSSRule> atRules;
   required unsigned long ruleCount;

layout/inspector/InspectorUtils.cpp

@@ -1002,21 +1002,10 @@ void InspectorUtils::GetCSSRegisteredProperties(
 }
 
 /* static */
-void InspectorUtils::GetRuleBodyTextOffsets(
-    GlobalObject&, const nsACString& aInitialText,
-    Nullable<InspectorGetRuleBodyTextResult>& aResult) {
-  uint32_t resultStartOffset;
-  uint32_t resultEndOffset;
-
-  if (!Servo_GetRuleBodyTextOffsets(&aInitialText, &resultStartOffset,
-                                    &resultEndOffset)) {
-    aResult.SetNull();
-    return;
-  }
-
-  InspectorGetRuleBodyTextResult& offsets = aResult.SetValue();
-  offsets.mStartOffset = resultStartOffset;
-  offsets.mEndOffset = resultEndOffset;
+void InspectorUtils::GetRuleBodyText(GlobalObject&,
+                                     const nsACString& aInitialText,
+                                     nsACString& aBodyText) {
+  Servo_GetRuleBodyText(&aInitialText, &aBodyText);
 }
 
 /* static */

layout/inspector/InspectorUtils.h

@@ -270,11 +270,10 @@ class InspectorUtils {
       nsTArray<InspectorCSSPropertyDefinition>& aResult);
 
   /**
-   * Get the rule body text start and end offsets within aInitialText
+   * Get the rule body text within aInitialText
    */
-  static void GetRuleBodyTextOffsets(
-      GlobalObject&, const nsACString& aInitialText,
-      Nullable<InspectorGetRuleBodyTextResult>& aResult);
+  static void GetRuleBodyText(GlobalObject&, const nsACString& aInitialText,
+                              nsACString& aBodyText);
 
   /**
    * Replace the rule body text in aStyleSheetText at passed line and column

servo/ports/geckolib/glue.rs

@@ -9121,23 +9121,20 @@ pub extern "C" fn Servo_GetSelectorWarnings(
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_GetRuleBodyTextOffsets(
+pub extern "C" fn Servo_GetRuleBodyText(
     initial_text: &nsACString,
-    result_start_offset: &mut u32,
-    result_end_offset: &mut u32,
-) -> bool {
+    ret_val: &mut nsACString,
+) {
     let css_text = unsafe { initial_text.as_str_unchecked() };
     let mut input = ParserInput::new(&css_text);
     let mut input = Parser::new(&mut input);
 
-    let mut start_offset = 0;
     let mut found_start = false;
 
     // Search forward for the opening brace.
     while let Ok(token) = input.next() {
         match *token {
             Token::CurlyBracketBlock => {
-                start_offset = input.position().byte_index();
                 found_start = true;
                 break;
             },
@@ -9145,13 +9142,14 @@ pub extern "C" fn Servo_GetRuleBodyTextOffsets(
         }
 
         if token.is_parse_error() {
-            return false;
+            break;
         }
     }
 
 
     if !found_start {
-        return false;
+        ret_val.set_is_void(true);
+        return;
     }
 
     let token_start = input.position();
@@ -9161,18 +9159,14 @@ pub extern "C" fn Servo_GetRuleBodyTextOffsets(
             Ok(())
         }
     );
-    let mut end_offset = input.position().byte_index();
+
     // We're not guaranteed to have a closing bracket, but when we do, we need to move
     // the end offset before it.
-    let token_slice = input.slice_from(token_start);
+    let mut token_slice = input.slice_from(token_start);
     if token_slice.ends_with("}") {
-        end_offset = end_offset - 1;
+        token_slice = token_slice.strip_suffix("}").unwrap();
     }
-
-    *result_start_offset = start_offset as u32;
-    *result_end_offset = end_offset as u32;
-
-    return true;
+    ret_val.assign(token_slice);
 }
 
 #[no_mangle]