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
authorJorg K <mozilla@jorgk.com>
Fri, 27 Mar 2015 16:16:05 -0400
changeset 236227 bf414f68291cd6a77b35de593ab589915db56b48
parent 236226 aca75f4ef930df34569d6d79b377a8d3186a5c24
child 236228 b2904e8f07e7995697039160f4846420e1fdb770
push id57632
push usereakhgari@mozilla.com
push dateSat, 28 Mar 2015 00:06:48 +0000
treeherdermozilla-inbound@bf414f68291c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, ehsan
bugs756984
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
editor/libeditor/tests/test_selection_move_commands.xul
layout/generic/nsFrame.cpp
layout/generic/test/mochitest.ini
layout/generic/test/test_bug756984.html
layout/generic/test/test_movement_by_characters.html
--- a/editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js
+++ b/editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js
@@ -517,25 +517,19 @@ const knownFailures = {
     "S-Proposed-SM:e.b.w_TEXT-1_SI-5-body": true,
     "S-Proposed-SM:e.b.w_TEXT-1_SI-5-div": true,
     "S-Proposed-SM:e.f.w_TEXT-1_SIR-1-dM": true,
     "S-Proposed-SM:e.f.w_TEXT-1_SIR-1-body": true,
     "S-Proposed-SM:e.f.w_TEXT-1_SIR-1-div": true,
     "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,
     "S-Proposed-SM:e.b.lb_BR.BR-1_SIR-2-dM": true,
     "S-Proposed-SM:e.b.lb_BR.BR-1_SIR-2-body": true,
     "S-Proposed-SM:e.b.lb_BR.BR-1_SIR-2-div": true,
     "S-Proposed-SM:e.b.lb_P.P.P-1_SIR-2-dM": true,
     "S-Proposed-SM:e.b.lb_P.P.P-1_SIR-2-body": true,
--- a/editor/libeditor/tests/test_selection_move_commands.xul
+++ b/editor/libeditor/tests/test_selection_move_commands.xul
@@ -166,19 +166,19 @@ function execTests() {
     doCommand("cmd_moveBottom");
     testMoveCommand("cmd_charPrevious", node(22), 1);
     testMoveCommand("cmd_charNext", node(22), 2);
     testSelectCommand("cmd_selectCharPrevious", node(22), 1);
     doCommand("cmd_moveTop");
     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);
 
     doCommand("cmd_moveBottom");
     testMoveCommand("cmd_wordPrevious", node(22), 0);
     testMoveCommand("cmd_wordNext", body, 23);
     testSelectCommand("cmd_selectWordPrevious", node(22), 0);
     doCommand("cmd_moveTop");
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -3543,20 +3543,26 @@ static FrameTarget GetSelectionClosestFr
   nsIFrame *frame = aLine->mFirstChild;
   // Account for end of lines (any iterator from the block is valid)
   if (aLine == aParent->end_lines())
     return DrillDownToSelectionFrame(aParent, true, aFlags);
   nsIFrame *closestFromIStart = nullptr, *closestFromIEnd = nullptr;
   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)) {
       if (pt.I(wm) < frameRect.IEnd(wm)) {
         return GetSelectionClosestFrameForChild(frame, aPoint, aFlags);
       }
       if (frameRect.IEnd(wm) >= closestIStart) {
         closestFromIStart = frame;
@@ -6790,16 +6796,22 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
         }
       } else {
         it->GetLine(thisLine, &firstFrame, &lineFrameCount, usedRect);
 
         nsIFrame* frame = firstFrame;
         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;
           }
         }
       }
       if (!baseFrame)
         return NS_ERROR_FAILURE;
@@ -7071,16 +7083,33 @@ nsIFrame::GetFrameFromDirection(nsDirect
 
     // Skip anonymous elements, but watch out for generated content
     if (!traversedFrame ||
         (!traversedFrame->IsGeneratedContentFrame() &&
          traversedFrame->GetContent()->IsRootOfNativeAnonymousSubtree())) {
       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;
     }
   } // while (!selectable)
 
   *aOutOffset = (aDirection == eDirNext) ? 0 : -1;
 
--- a/layout/generic/test/mochitest.ini
+++ b/layout/generic/test/mochitest.ini
@@ -80,16 +80,17 @@ support-files = bug633762_iframe.html
 skip-if = buildapp == 'mulet' # TC: Bug 1144079 - Re-enable Mulet mochitests and reftests taskcluster-specific disables.
 [test_bug719503.html]
 [test_bug719515.html]
 [test_bug719518.html]
 [test_bug719523.html]
 [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]
 [test_bug791616.html]
 skip-if = buildapp == 'b2g' # b2g(Target should not have scrolled - got 114.10000610351562, expected 115.39999389648438) b2g-debug(Target should not have scrolled - got 114.10000610351562, expected 115.39999389648438) b2g-desktop(Target should not have scrolled - got 114.10000610351562, expected 115.39999389648438)
 [test_bug831780.html]
 [test_bug841361.html]
 [test_bug904810.html]
new file mode 100644
--- /dev/null
+++ b/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>
--- a/layout/generic/test/test_movement_by_characters.html
+++ b/layout/generic/test/test_movement_by_characters.html
@@ -65,17 +65,17 @@ function test() {
 
   editor.innerHTML = "H <br>K";
   sel.collapse(editor.firstChild, 0);
   testRight(editor.firstChild, 1);
   testRight(editor.firstChild, 2);
   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);
 
   editor.innerHTML = "<pre>aa\nbb</pre>";
   sel.collapse(editor.firstChild.firstChild, 0);
   testRight(editor.firstChild.firstChild, 1);
   // at the end of the first line, before the \n
   testRight(editor.firstChild.firstChild, 2);