Bug 1879806 - [devtools] Only apply unicode-bidi: isolate to relevant…

… elements. r=devtools-reviewers,ochameau.

The CSS property was applied broadly on all element with
a objectBox parent, which was bad for performance.
We now use `Services.intl.stringHasRTLChars` to detect if the string we're going
to render has some RTL chars, and in such case, we add a `has-rtl-char` class
to the element so we can target it in CSS.

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

devtools/client/shared/components/reps/reps.css

@@ -37,7 +37,13 @@
   white-space: pre-wrap;
 }
 
-.objectBox * {
+:is(
+  .objectBox-string,
+  .objectBox-textNode,
+  .objectBox > .nodeName,
+  .objectBox-node .tag-name,
+  .objectBox-node .attrName
+).has-rtl-char {
   unicode-bidi: isolate;
 }
 

devtools/client/shared/components/reps/reps/attribute.js

@@ -12,6 +12,7 @@ define(function (require, exports, module) {
 
   // Reps
   const {
+    appendRTLClassNameIfNeeded,
     getGripType,
     wrapRender,
   } = require("devtools/client/shared/components/reps/reps/rep-utils");
@@ -42,7 +43,12 @@ define(function (require, exports, module) {
 
     return span(
       config,
-      span({ className: "attrName" }, attrName),
+      span(
+        {
+          className: appendRTLClassNameIfNeeded("attrName", attrName),
+        },
+        attrName
+      ),
       span({ className: "attrEqual" }, "="),
       StringRep({ className: "attrValue", object: value })
     );

devtools/client/shared/components/reps/reps/element-node.js

@@ -15,6 +15,7 @@ define(function (require, exports, module) {
 
   // Utils
   const {
+    appendRTLClassNameIfNeeded,
     wrapRender,
   } = require("devtools/client/shared/components/reps/reps/rep-utils");
   const {
@@ -165,7 +166,7 @@ define(function (require, exports, module) {
 
     const nodeNameElement = span(
       {
-        className: "tag-name",
+        className: appendRTLClassNameIfNeeded("tag-name", nodeName),
       },
       nodeName
     );
@@ -189,7 +190,12 @@ define(function (require, exports, module) {
 
       const attribute = span(
         {},
-        span({ className: "attrName" }, name),
+        span(
+          {
+            className: appendRTLClassNameIfNeeded("attrName", name),
+          },
+          name
+        ),
         span({ className: "attrEqual" }, "="),
         StringRep({
           className: "attrValue",
@@ -216,15 +222,19 @@ define(function (require, exports, module) {
     // Initialize elements array
     const elements = [
       {
-        config: { className: "tag-name" },
+        config: {
+          className: appendRTLClassNameIfNeeded("tag-name", nodeName),
+        },
         content: nodeName,
       },
     ];
 
     // Push ID element
     if (attributes.id) {
       elements.push({
-        config: { className: "attrName" },
+        config: {
+          className: appendRTLClassNameIfNeeded("attrName", attributes.id),
+        },
         content: `#${attributes.id}`,
       });
     }
@@ -237,7 +247,9 @@ define(function (require, exports, module) {
         .map(cls => `.${cls}`)
         .join("");
       elements.push({
-        config: { className: "attrName" },
+        config: {
+          className: appendRTLClassNameIfNeeded("attrName", elementClasses),
+        },
         content: elementClasses,
       });
     }

devtools/client/shared/components/reps/reps/prop-rep.js

@@ -11,6 +11,7 @@ define(function (require, exports, module) {
   const { span } = require("devtools/client/shared/vendor/react-dom-factories");
 
   const {
+    appendRTLClassNameIfNeeded,
     maybeEscapePropertyName,
     wrapRender,
   } = require("devtools/client/shared/components/reps/reps/rep-utils");
@@ -73,7 +74,7 @@ define(function (require, exports, module) {
       }
       key = span(
         {
-          className,
+          className: appendRTLClassNameIfNeeded(className, name),
           title: shouldRenderTooltip ? name : null,
         },
         name

devtools/client/shared/components/reps/reps/rep-utils.js

@@ -548,12 +548,30 @@ define(function (require, exports, module) {
       }, {});
   }
 
+  /**
+   * Append has-rtl-char to className if passed string has RTL chars.
+   * has-rtl-char is used in reps.css to set `unicode-bidi: isolate` on the element.
+   * It's important to only apply it when needed as this CSS property can have an
+   * important impact on performance (See Bug 1879806)
+   *
+   * @param {String} className: The className want to set on an element
+   * @param {String} strToCheck: The string for which we want to check if it has RTL chars
+   * @returns {String}
+   */
+  function appendRTLClassNameIfNeeded(className = "", strToCheck) {
+    if (!Services.intl.stringHasRTLChars(strToCheck)) {
+      return className;
+    }
+    return `${className} has-rtl-char`;
+  }
+
   module.exports = {
     interleave,
     isURL,
     cropString,
     containsURL,
     rawCropString,
+    appendRTLClassNameIfNeeded,
     sanitizeString,
     escapeString,
     wrapRender,

devtools/client/shared/components/reps/reps/string.js

@@ -14,6 +14,7 @@ define(function (require, exports, module) {
   const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
   const {
+    appendRTLClassNameIfNeeded,
     containsURL,
     escapeString,
     getGripType,
@@ -136,6 +137,8 @@ define(function (require, exports, module) {
       );
     }
 
+    config.className = appendRTLClassNameIfNeeded(config.className, text);
+
     return span(config, text);
   }
 

devtools/client/shared/components/reps/reps/text-node.js

@@ -15,6 +15,7 @@ define(function (require, exports, module) {
 
   // Reps
   const {
+    appendRTLClassNameIfNeeded,
     cropString,
     wrapRender,
   } = require("devtools/client/shared/components/reps/reps/rep-utils");
@@ -71,13 +72,17 @@ define(function (require, exports, module) {
       shouldRenderTooltip,
     } = opts;
 
+    const text = getTextContent(object);
     const config = {
       "data-link-actor-id": object.actor,
       "data-link-content-dom-reference": JSON.stringify(
         object.contentDomReference
       ),
-      className: "objectBox objectBox-textNode",
-      title: shouldRenderTooltip ? `#text "${getTextContent(object)}"` : null,
+      className: appendRTLClassNameIfNeeded(
+        "objectBox objectBox-textNode",
+        text
+      ),
+      title: shouldRenderTooltip ? `#text "${text}"` : null,
     };
 
     if (isInTree) {

devtools/client/shared/test-helpers/jest-fixtures/Services.js

@@ -549,6 +549,9 @@ const Services = {
       };
     },
   },
+  intl: {
+    stringHasRTLChars: () => false,
+  },
 };
 
 function pref(name, value) {