From 8de2c3cb192fc2fe69cf155634125454e820da63 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 13 Jul 2010 21:49:16 -0400 Subject: [PATCH] Bug 240933 - Part 3: Correct the caret movement throughout textareas (and pre elements with caret browsing turned on as well); r=roc a=dbaron --HG-- extra : rebase_source : 9f015607bc84a11137ab11ba47c14e98c20e3970 --- layout/generic/nsFrame.cpp | 11 +++- layout/generic/nsTextFrameThebes.cpp | 30 ++++----- layout/generic/test/test_bug288789.html | 61 ++++++++++++++++++- .../test/test_movement_by_characters.html | 8 +-- 4 files changed, 88 insertions(+), 22 deletions(-) diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 5ce73aedcaaf..4c1539efdcd0 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -5348,6 +5348,7 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos) { PRBool eatingNonRenderableWS = PR_FALSE; PRBool done = PR_FALSE; + PRBool jumpedLine = PR_FALSE; while (!done) { PRBool movingInFrameDirection = @@ -5359,7 +5360,6 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos) done = current->PeekOffsetCharacter(movingInFrameDirection, &offset); if (!done) { - PRBool jumpedLine; result = current->GetFrameFromDirection(aPos->mDirection, aPos->mVisual, aPos->mJumpLines, aPos->mScrollViewStop, @@ -5380,6 +5380,15 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos) aPos->mResultContent = range.content; // Output offset is relative to content, not frame aPos->mContentOffset = offset < 0 ? range.end : range.start + offset; + // If we're dealing with a text frame and moving backward positions us at + // the end of that line, decrease the offset by one to make sure that + // we're placed before the linefeed character on the previous line. + if (offset < 0 && jumpedLine && + aPos->mDirection == eDirPrevious && + current->GetStyleText()->NewlineIsSignificant() && + current->HasTerminalNewline()) { + --aPos->mContentOffset; + } break; } diff --git a/layout/generic/nsTextFrameThebes.cpp b/layout/generic/nsTextFrameThebes.cpp index 7f698d207d94..c10c92ec1361 100644 --- a/layout/generic/nsTextFrameThebes.cpp +++ b/layout/generic/nsTextFrameThebes.cpp @@ -5425,8 +5425,7 @@ IsAcceptableCaretPosition(const gfxSkipCharsIterator& aIter, gfxTextRun* aTextRu PRUint32 index = aIter.GetSkippedOffset(); if (!aTextRun->IsClusterStart(index)) return PR_FALSE; - return !(aFrame->GetStyleText()->NewlineIsSignificant() && - aTextRun->GetChar(index) == '\n'); + return PR_TRUE; } PRBool @@ -5451,8 +5450,8 @@ nsTextFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset) PRInt32 startOffset = GetContentOffset() + (*aOffset < 0 ? contentLength : *aOffset); if (!aForward) { - PRInt32 i; - for (i = NS_MIN(trimmed.GetEnd(), startOffset) - 1; + // If at the beginning of the line, look at the previous continuation + for (PRInt32 i = NS_MIN(trimmed.GetEnd(), startOffset) - 1; i >= trimmed.mStart; --i) { iter.SetOriginalOffset(i); if (IsAcceptableCaretPosition(iter, mTextRun, this)) { @@ -5462,16 +5461,19 @@ nsTextFrame::PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset) } *aOffset = 0; } else { - PRInt32 i; - for (i = startOffset + 1; i <= trimmed.GetEnd(); ++i) { - iter.SetOriginalOffset(i); - // XXX we can't necessarily stop at the end of this frame, - // but we really have no choice right now. We need to do a deeper - // fix/restructuring of PeekOffsetCharacter - if (i == trimmed.GetEnd() || - IsAcceptableCaretPosition(iter, mTextRun, this)) { - *aOffset = i - mContentOffset; - return PR_TRUE; + // If we're at the end of a line, look at the next continuation + iter.SetOriginalOffset(startOffset); + if (iter.GetSkippedOffset() <= PRUint32(trimmed.GetEnd()) && + !(iter.GetSkippedOffset() < PRUint32(trimmed.GetEnd()) && + GetStyleText()->NewlineIsSignificant() && + mTextRun->GetChar(iter.GetSkippedOffset()) == '\n')) { + for (PRInt32 i = startOffset + 1; i <= trimmed.GetEnd(); ++i) { + iter.SetOriginalOffset(i); + if (i == trimmed.GetEnd() || + IsAcceptableCaretPosition(iter, mTextRun, this)) { + *aOffset = i - mContentOffset; + return PR_TRUE; + } } } *aOffset = contentLength; diff --git a/layout/generic/test/test_bug288789.html b/layout/generic/test/test_bug288789.html index 5e2f15f6272c..e21f2d4ee722 100644 --- a/layout/generic/test/test_bug288789.html +++ b/layout/generic/test/test_bug288789.html @@ -18,6 +18,11 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=288789 אaב + + 
@@ -50,9 +55,59 @@ function test() {
 
   textarea.focus();
   collapse(0);
-  testLeft(1);
-  collapse(5);
-  testRight(4);
+  ok(true, "Testing forward movement in RTL mode");
+  for (var i = 0; i < textarea.textContent.length; ++i) {
+    if (i == 0) {
+      testRight(i);
+    }
+    if (textarea.textContent[i] == 'a') {
+      testLeft(i);
+    } else {
+      testLeft(i + 1);
+    }
+    if (i == textarea.textContent.length - 1) {
+      testLeft(i + 1);
+    }
+  }
+  ok(true, "Testing backward movement in RTL mode");
+  for (var i = textarea.textContent.length; i > 0; --i) {
+    if (i == textarea.textContent.length) {
+      testLeft(i);
+    }
+    if (i > 0 && textarea.textContent[i - 1] == 'a') {
+      testRight(i);
+    } else {
+      testRight(i - 1);
+    }
+    if (i == 1) {
+      testRight(i - 1);
+    }
+  }
+
+  textarea = $("tb");
+  textarea.focus();
+  collapse(0);
+  ok(true, "Testing forward movement in LTR mode");
+  for (var i = 0; i < textarea.textContent.length; ++i) {
+    if (i == 0) {
+      testLeft(i);
+    }
+    testRight(i + 1);
+    if (i == textarea.textContent.length - 1) {
+      testRight(i + 1);
+    }
+  }
+  ok(true, "Testing backward movement in LTR mode");
+  for (var i = textarea.textContent.length; i > 0; --i) {
+    if (i == textarea.textContent.length) {
+      testRight(i);
+    }
+    testLeft(i - 1);
+    if (i == 1) {
+      testLeft(i - 1);
+    }
+  }
+
   SimpleTest.finish();
 }
 
diff --git a/layout/generic/test/test_movement_by_characters.html b/layout/generic/test/test_movement_by_characters.html
index b8a0c4f616eb..61fff0865e08 100644
--- a/layout/generic/test/test_movement_by_characters.html
+++ b/layout/generic/test/test_movement_by_characters.html
@@ -68,13 +68,13 @@ function test() {
   editor.innerHTML = "
aa\nbb
";
   sel.collapse(editor.firstChild.firstChild, 0);
   testRight(editor.firstChild.firstChild, 1);
-  // at the 'bb' but HINTLEFT so appears at the end of the first line
-  testRight(editor.firstChild.firstChild, 3);  
+  // at the end of the first line, before the \n
+  testRight(editor.firstChild.firstChild, 2);
   testRight(editor.firstChild.firstChild, 3);
   testRight(editor.firstChild.firstChild, 4);
   testLeft(editor.firstChild.firstChild, 3);
-  // at the 'bb' but HINTLEFT so appears at the end of the first line
-  testLeft(editor.firstChild.firstChild, 3);
+  // at the end of the first line, before the \n
+  testLeft(editor.firstChild.firstChild, 2);
   testLeft(editor.firstChild.firstChild, 1);
   testLeft(editor.firstChild.firstChild, 0);