Bug 1927404 - Make each line in view-source a <span> rather than a <pre>. r=hsivonen

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

devtools/startup/tests/browser/browser_command_line_urls.js

@@ -90,7 +90,7 @@ add_task(async function openingWithDevToolsButUnknownSource() {
 
     const selection = content.getSelection();
     Assert.equal(
-      selection.toString(),
+      selection.toString().trimEnd(),
       "  <title>Command line test page</title>",
       "The 5th line is selected in view-source"
     );

layout/style/res/viewsource.css

@@ -29,17 +29,20 @@ pre {
   font: inherit;
   color: inherit;
   white-space: inherit;
-  margin: 0 0 0 5ch;
 }
 
-pre[id]::before,
+span[id] {
+  display: block;
+  margin-left: 5ch;
+}
+
 span[id]::before {
   content: counter(line) " ";
   counter-increment: line;
   user-select: none;
   display: inline-block;
   width: 5ch;
-  margin: 0 0 0 -5ch;
+  margin-left: -5ch;
   text-align: right;
   color: #ccc;
   font-weight: normal;
@@ -94,7 +97,3 @@ span:not(.error),
 a:not(.error) {
   unicode-bidi: embed;
 }
-
-span[id] {
-  unicode-bidi: isolate;
-}

parser/html/nsHtml5Highlighter.cpp

@@ -500,7 +500,7 @@ void nsHtml5Highlighter::EndSpanOrA() {
 
 void nsHtml5Highlighter::StartBodyContents() {
   MOZ_ASSERT(mLineNumber == 1);
-  PushCurrentLinePre();
+  PushCurrentLineContainer();
   StartCharacters();
 }
 
@@ -577,8 +577,8 @@ void nsHtml5Highlighter::FlushChars() {
   }
 }
 
-void nsHtml5Highlighter::PushCurrentLinePre() {
-  Push(nsGkAtoms::pre, nullptr, NS_NewHTMLPreElement);
+void nsHtml5Highlighter::PushCurrentLineContainer() {
+  Push(nsGkAtoms::span, nullptr, NS_NewHTMLSpanElement);
   mOpQueue.AppendElement()->Init(
       mozilla::AsVariant(opAddLineNumberId(CurrentNode(), mLineNumber)));
 }
@@ -598,8 +598,8 @@ void nsHtml5Highlighter::NewLine() {
     Pop();
     mInlinesOpen--;
   }
-  Pop();  // Pop the <pre>
-  PushCurrentLinePre();
+  Pop();  // Pop the existing container.
+  PushCurrentLineContainer();
   for (nsIContent** handle : Reversed(handleStack)) {
     nsIContent** dest = AllocateContentHandle();
     mOpQueue.AppendElement()->Init(mozilla::AsVariant(opShallowCloneInto(

parser/html/nsHtml5Highlighter.h

@@ -277,8 +277,8 @@ class nsHtml5Highlighter {
   void Push(nsAtom* aName, nsHtml5HtmlAttributes* aAttributes,
             mozilla::dom::HTMLContentCreatorFunction aCreator);
 
-  /** Pushes a <pre id="line<lineno>"> */
-  void PushCurrentLinePre();
+  /** Pushes a <span id="line<lineno>"> */
+  void PushCurrentLineContainer();
 
   /**
    * Pops all inlines from the stack, pushes a pre, and pushes all inlines back

parser/html/nsHtml5ViewSourceUtils.cpp

@@ -3,7 +3,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsHtml5ViewSourceUtils.h"
-#include "mozilla/Preferences.h"
 #include "nsHtml5AttributeName.h"
 #include "nsHtml5String.h"
 #include "mozilla/StaticPrefs_view_source.h"

toolkit/actors/ViewSourcePageChild.sys.mjs

@@ -143,69 +143,15 @@ export class ViewSourcePageChild extends JSWindowActorChild {
    *        The line number to attempt to go to.
    */
   goToLine(lineNumber) {
-    let body = this.document.body;
-
-    // The source document is made up of a number of pre elements with
-    // id attributes in the format <pre id="line123">, meaning that
-    // the first line in the pre element is number 123.
-    // Do binary search to find the pre element containing the line.
-    // However, in the plain text case, we have only one pre without an
-    // attribute, so assume it begins on line 1.
-    let pre;
-    for (let lbound = 0, ubound = body.childNodes.length; ; ) {
-      let middle = (lbound + ubound) >> 1;
-      pre = body.childNodes[middle];
-
-      let firstLine = pre.id ? parseInt(pre.id.substring(4)) : 1;
-
-      if (lbound == ubound - 1) {
-        break;
-      }
-
-      if (lineNumber >= firstLine) {
-        lbound = middle;
-      } else {
-        ubound = middle;
-      }
-    }
-
-    let result = {};
-    let found = this.findLocation(pre, lineNumber, null, -1, false, result);
-
-    if (!found) {
+    let range = this.findLocation(lineNumber);
+    if (!range) {
       this.sendAsyncMessage("ViewSource:GoToLine:Failed");
       return;
     }
 
     let selection = this.document.defaultView.getSelection();
     selection.removeAllRanges();
-
-    // In our case, the range's startOffset is after "\n" on the previous line.
-    // Tune the selection at the beginning of the next line and do some tweaking
-    // to position the focusNode and the caret at the beginning of the line.
-    selection.interlinePosition = true;
-
-    selection.addRange(result.range);
-
-    if (!selection.isCollapsed) {
-      selection.collapseToEnd();
-
-      let offset = result.range.startOffset;
-      let node = result.range.startContainer;
-      if (offset < node.data.length) {
-        // The same text node spans across the "\n", just focus where we were.
-        selection.extend(node, offset);
-      } else {
-        // There is another tag just after the "\n", hook there. We need
-        // to focus a safe point because there are edgy cases such as
-        // <span>...\n</span><span>...</span> vs.
-        // <span>...\n<span>...</span></span><span>...</span>
-        node = node.nextSibling
-          ? node.nextSibling
-          : node.parentNode.nextSibling;
-        selection.extend(node, 0);
-      }
-    }
+    selection.addRange(range);
 
     let selCon = this.selectionController;
     selCon.setDisplaySelection(Ci.nsISelectionController.SELECTION_ON);
@@ -221,42 +167,28 @@ export class ViewSourcePageChild extends JSWindowActorChild {
     this.sendAsyncMessage("ViewSource:GoToLine:Success", { lineNumber });
   }
 
-  /**
-   * Some old code from the original view source implementation. Original
-   * documentation follows:
-   *
-   * "Loops through the text lines in the pre element. The arguments are either
-   *  (pre, line) or (node, offset, interlinePosition). result is an out
-   *  argument. If (pre, line) are specified (and node == null), result.range is
-   *  a range spanning the specified line. If the (node, offset,
-   *  interlinePosition) are specified, result.line and result.col are the line
-   *  and column number of the specified offset in the specified node relative to
-   *  the whole file."
-   */
-  findLocation(pre, lineNumber, node, offset, interlinePosition, result) {
-    if (node && !pre) {
-      // Look upwards to find the current pre element.
-      // eslint-disable-next-line no-empty
-      for (pre = node; pre.nodeName != "PRE"; pre = pre.parentNode) {}
+  findLocation(lineNumber) {
+    let line = this.document.getElementById(`line${lineNumber}`);
+    let range = null;
+    if (line) {
+      range = this.document.createRange();
+      range.setStart(line, 0);
+      range.setEndAfter(line, line.childNodes.length);
+      return range;
     }
 
-    // The source document is made up of a number of pre elements with
-    // id attributes in the format <pre id="line123">, meaning that
-    // the first line in the pre element is number 123.
-    // However, in the plain text case, there is only one <pre> without an id,
-    // so assume line 1.
-    let curLine = pre.id ? parseInt(pre.id.substring(4)) : 1;
-
+    let pre = this.document.querySelector("pre");
+    if (pre.id) {
+      return null;
+    }
     // Walk through each of the text nodes and count newlines.
+    let curLine = 1;
     let treewalker = this.document.createTreeWalker(
       pre,
       NodeFilter.SHOW_TEXT,
       null
     );
 
-    // The column number of the first character in the current text node.
-    let firstCol = 1;
-
     let found = false;
     for (
       let textNode = treewalker.firstChild();
@@ -268,11 +200,7 @@ export class ViewSourcePageChild extends JSWindowActorChild {
       let lastLineInNode = curLine + lineArray.length - 1;
 
       // Check if we can skip the text node without further inspection.
-      if (node ? textNode != node : lastLineInNode < lineNumber) {
-        if (lineArray.length > 1) {
-          firstCol = 1;
-        }
-        firstCol += lineArray[lineArray.length - 1].length;
+      if (lastLineInNode < lineNumber) {
         curLine = lastLineInNode;
         continue;
       }
@@ -288,41 +216,22 @@ export class ViewSourcePageChild extends JSWindowActorChild {
           curLine++;
         }
 
-        if (node) {
-          if (offset >= curPos && offset <= curPos + lineArray[i].length) {
-            // If we are right after the \n of a line and interlinePosition is
-            // false, the caret looks as if it were at the end of the previous
-            // line, so we display that line and column instead.
-
-            if (i > 0 && offset == curPos && !interlinePosition) {
-              result.line = curLine - 1;
-              var prevPos = curPos - lineArray[i - 1].length;
-              result.col = (i == 1 ? firstCol : 1) + offset - prevPos;
-            } else {
-              result.line = curLine;
-              result.col = (i == 0 ? firstCol : 1) + offset - curPos;
-            }
-            found = true;
-
-            break;
-          }
-        } else if (curLine == lineNumber && !("range" in result)) {
-          result.range = this.document.createRange();
-          result.range.setStart(textNode, curPos);
+        if (curLine == lineNumber && !range) {
+          range = this.document.createRange();
+          range.setStart(textNode, curPos);
 
           // This will always be overridden later, except when we look for
           // the very last line in the file (this is the only line that does
           // not end with \n).
-          result.range.setEndAfter(pre.lastChild);
+          range.setEndAfter(pre.lastChild);
         } else if (curLine == lineNumber + 1) {
-          result.range.setEnd(textNode, curPos - 1);
-          found = true;
+          range.setEnd(textNode, curPos - 1);
           break;
         }
       }
     }
 
-    return found || "range" in result;
+    return range;
   }
 
   /**

toolkit/components/viewsource/test/browser/browser_contextmenu.js

@@ -54,7 +54,7 @@ async function onViewSourceWindowOpen(aWindow) {
 
   expectedData.push(["a[href]", true, false, "http://example.com/"]);
   expectedData.push(["a[href^=mailto]", false, true, "abc@def.ghi"]);
-  expectedData.push(["span", false, false, null]);
+  expectedData.push(["span:not([id])", false, false, null]);
 }
 
 async function checkMenuItems(

toolkit/components/viewsource/test/browser/browser_gotoline.js

@@ -31,7 +31,11 @@ var checkViewSource = async function (aTab) {
     );
     await SpecialPowers.spawn(browser, [i], async function (i) {
       let selection = content.getSelection();
-      Assert.equal(selection.toString(), "line " + i, "Correct text selected");
+      Assert.equal(
+        selection.toString().trim(),
+        "line " + i,
+        "Correct text selected"
+      );
     });
   }
 };