Bug 756984 - Collapse the selection on the last text node on the line, skipping br and inline frames when clicking past the end of line; r=roc,ehsan

editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js

@@ -522,15 +522,9 @@ const knownFailures = {
     "S-Proposed-SM:e.f.w_TEXT-1_SIR-3-dM": true,
     "S-Proposed-SM:e.f.w_TEXT-1_SIR-3-body": true,
     "S-Proposed-SM:e.f.w_TEXT-1_SIR-3-div": true,
-    "S-Proposed-SM:e.f.lb_BR.BR-1_SC-1-dM": true,
-    "S-Proposed-SM:e.f.lb_BR.BR-1_SC-1-body": true,
-    "S-Proposed-SM:e.f.lb_BR.BR-1_SC-1-div": true,
     "S-Proposed-SM:e.f.lb_BR.BR-1_SI-1-dM": true,
     "S-Proposed-SM:e.f.lb_BR.BR-1_SI-1-body": true,
     "S-Proposed-SM:e.f.lb_BR.BR-1_SI-1-div": true,
-    "S-Proposed-SM:e.f.lb_BR.BR-1_SM-1-dM": true,
-    "S-Proposed-SM:e.f.lb_BR.BR-1_SM-1-body": true,
-    "S-Proposed-SM:e.f.lb_BR.BR-1_SM-1-div": true,
     "S-Proposed-SM:e.f.lb_P.P.P-1_SI-1-dM": true,
     "S-Proposed-SM:e.f.lb_P.P.P-1_SI-1-body": true,
     "S-Proposed-SM:e.f.lb_P.P.P-1_SI-1-div": true,

editor/libeditor/tests/test_selection_move_commands.xul

@@ -171,9 +171,9 @@ function execTests() {
     testSelectCommand("cmd_selectCharNext", node(0), 1);
 
     doCommand("cmd_moveTop");
-    testMoveCommand("cmd_endLine", body, 1);
+    testMoveCommand("cmd_endLine", node(0), 1);
     testMoveCommand("cmd_beginLine", node(0), 0);
-    testSelectCommand("cmd_selectEndLine", body, 1);
+    testSelectCommand("cmd_selectEndLine", node(0), 1);
     doCommand("cmd_moveBottom");
     testSelectCommand("cmd_selectBeginLine", node(22), 0);
 

layout/generic/nsFrame.cpp

@@ -3548,10 +3548,16 @@ static FrameTarget GetSelectionClosestFrameForLine(
   nscoord closestIStart = aLine->IStart(), closestIEnd = aLine->IEnd();
   WritingMode wm = aLine->mWritingMode;
   LogicalPoint pt(wm, aPoint, aLine->mContainerWidth);
+  bool canSkipBr = false;
   for (int32_t n = aLine->GetChildCount(); n;
        --n, frame = frame->GetNextSibling()) {
-    if (!SelfIsSelectable(frame, aFlags) || frame->IsEmpty())
+    // Skip brFrames. Can only skip if the line contains at least
+    // one selectable and non-empty frame before
+    if (!SelfIsSelectable(frame, aFlags) || frame->IsEmpty() ||
+        (canSkipBr && frame->GetType() == nsGkAtoms::brFrame)) {
       continue;
+    }
+    canSkipBr = true;
     LogicalRect frameRect = LogicalRect(wm, frame->GetRect(),
                                         aLine->mContainerWidth);
     if (pt.I(wm) >= frameRect.IStart(wm)) {
@@ -6795,6 +6801,12 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos)
         for (int32_t count = lineFrameCount; count;
              --count, frame = frame->GetNextSibling()) {
           if (!frame->IsGeneratedContentFrame()) {
+            // When jumping to the end of the line with the "end" key,
+            // skip over brFrames
+            if (endOfLine && lineFrameCount > 1 &&
+                frame->GetType() == nsGkAtoms::brFrame) {
+              continue;
+            }
             baseFrame = frame;
             if (!endOfLine)
               break;
@@ -7076,6 +7088,23 @@ nsIFrame::GetFrameFromDirection(nsDirection aDirection, bool aVisual,
       return NS_ERROR_FAILURE;
     }
 
+    // Skip brFrames, but only if they are not the only frame in the line
+    if (atLineEdge && aDirection == eDirPrevious &&
+        traversedFrame->GetType() == nsGkAtoms::brFrame) {
+      int32_t lineFrameCount;
+      nsIFrame *currentBlockFrame, *currentFirstFrame;
+      nsRect usedRect;
+      int32_t currentLine = nsFrame::GetLineNumber(traversedFrame, aScrollViewStop, &currentBlockFrame);
+      nsAutoLineIterator iter = currentBlockFrame->GetLineIterator();
+      result = iter->GetLine(currentLine, &currentFirstFrame, &lineFrameCount, usedRect);
+      if (NS_FAILED(result)) {
+        return result;
+      }
+      if (lineFrameCount > 1) {
+        continue;
+      }
+    }
+
     traversedFrame->IsSelectable(&selectable, nullptr);
     if (!selectable) {
       *aOutMovedOverNonSelectableText = true;

layout/generic/test/mochitest.ini

@@ -85,6 +85,7 @@ skip-if = buildapp == 'mulet' # TC: Bug 1144079 - Re-enable Mulet mochitests and
 [test_bug735641.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage
 [test_bug748961.html]
+[test_bug756984.html]
 [test_bug784410.html]
 skip-if = buildapp == 'b2g' #Bug 931116, 1129060 no wheel events on b2g
 [test_bug785324.html]

layout/generic/test/test_bug756984.html

@@ -0,0 +1,66 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=756984
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 756984</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=756984">Mozilla Bug 756984</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+
+<div id="div1">123<br>45678<br></div>
+<div id="div2"><font face="Arial">123</font><br><i>45678</i><br></div>
+<div id="div3"><font face="Courier"><i><strong>123</strong></i></font><br><i>45678</i><br></div>
+<div id="div4"><br>45678<br></div>
+
+<pre id="test">
+
+ <script type="application/javascript">
+
+    /** Test for Bug 756984 **/
+    /** We test that clicking beyond the end of a line terminated with <br> selects the preceding text, if any **/
+
+    SimpleTest.waitForExplicitFinish();
+
+    SimpleTest.waitForFocus(function() {
+
+    var sel = window.getSelection();
+    
+    for (i = 1; i <= 3; i++) {
+        // click beyond the first line (100px to the left and 2px down), expect text
+        var theDiv = document.getElementById("div" + i.toString());
+        theDiv.focus();
+        sel.collapse(theDiv, 0);
+        synthesizeMouse(theDiv, 100, 2, {});
+        var selRange = sel.getRangeAt(0);
+        is(selRange.endContainer.nodeName, "#text", "selection should be in text node");
+        is(selRange.endOffset, 3, "offset should be 3");
+      }
+
+      // click beyond the first line (100px to the left and 2px down), expect DIV.
+      // This is the previous behaviour which hasn't changed since the line is empty.
+      // If the processing were wrong, the selection would end up in some other non-empty line.
+      theDiv = document.getElementById("div4");
+      theDiv.focus();
+      sel.collapse(theDiv, 0);
+      synthesizeMouse(theDiv, 100, 2, {});
+      selRange = sel.getRangeAt(0);
+      is(selRange.endContainer.nodeName, "DIV", "selection should be in DIV");
+      is(selRange.endOffset, 0, "offset should be 0");
+
+      SimpleTest.finish();
+    });
+
+  </script>
+
+</pre>
+</body>
+</html>

layout/generic/test/test_movement_by_characters.html

@@ -70,7 +70,7 @@ function test() {
   testRight(editor.firstChild.nextSibling.nextSibling, 0);
   testRight(editor.firstChild.nextSibling.nextSibling, 1);
   testLeft(editor.firstChild.nextSibling.nextSibling, 0);
-  testLeft(editor, 1);
+  testLeft(editor.firstChild, 2);
   testLeft(editor.firstChild, 1);
   testLeft(editor.firstChild, 0);