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 THUNDERBIRD_38_VERBRANCH
authorJorg K <mozilla@jorgk.com>
Fri, 27 Mar 2015 16:16:05 -0400
branchTHUNDERBIRD_38_VERBRANCH
changeset 260324 2b80e0ec7f3534eff1fee610fd961d65a6efeda3
parent 260323 ebf8c3e088fc3c9663f32a7d144e63c1be40ed7e
child 260325 222192907d835cc7f703dbf6061df23b8c6953cd
push id749
push usermozilla@kewis.ch
push dateWed, 29 Apr 2015 19:45:09 +0000
treeherdermozilla-release@222192907d83 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, ehsan
bugs756984
milestone38.0
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
@@ -3534,20 +3534,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;
@@ -6781,16 +6787,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;
@@ -7062,16 +7074,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
@@ -77,16 +77,17 @@ support-files = bug633762_iframe.html
 [test_bug666225.html]
 [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' && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage
 [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);