Bug 989012 - Part 1: Stop after passing over a non-selectable frame if one is found during the frame traversal; r=roc

The caret movement code already handles unselectable text frames if we
happen to land in the middle of one in nsTextFrame::PeekOffsetCharacter/Word.
However, when performing frame traversal to find the next frame to jump
to, we don't remember if we skipped over an unselectable frame, which causes
us to jump one offset too much when the caret is on the boundary of
selectable and unselectable content.  The test cases demonstrate the
scenario.  Note that an <img alt=foo> is implemented by adding a
generated content to the inline frame representing it, so as far as
the caret movement code is concerned, both test cases are treated similarly.

Note that we need to do this only when moving the selection, and not
when extending it.  We are adding an aExtend argument to
nsPeekOffsetStruct's constructor in order to be able to special case
that.
This commit is contained in:
Ehsan Akhgari 2015-01-15 11:24:49 -05:00
parent 0bd29f0e96
commit 0cbca8d56b
13 changed files with 153 additions and 17 deletions

View File

@ -510,7 +510,7 @@ HyperTextAccessible::FindOffset(uint32_t aOffset, nsDirection aDirection,
nsPeekOffsetStruct pos(aAmount, aDirection, innerContentOffset,
nsPoint(0, 0), kIsJumpLinesOk, kIsScrollViewAStop,
kIsKeyboardSelect, kIsVisualBidi,
aWordMovementType);
false, aWordMovementType);
nsresult rv = frameAtOffset->PeekOffset(&pos);
// PeekOffset fails on last/first lines of the text in certain cases.

View File

@ -697,6 +697,7 @@ CompareRangeWithContentOffset(nsRange* aRange,
true,
true, //limit on scrolled views
false,
false,
false);
nsresult rv = theFrame->PeekOffset(&pos);
if (NS_FAILED(rv)) {

View File

@ -702,7 +702,7 @@ nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection,
{
nsPeekOffsetStruct pos(eSelectBeginLine, eDirPrevious, 0,
nsPoint(0, 0), false, true, false,
true);
true, false);
if (NS_SUCCEEDED(frameAfter->PeekOffset(&pos))) {
theFrame = pos.mResultFrame;
theFrameOffset = pos.mContentOffset;
@ -737,7 +737,7 @@ nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection,
{
nsPeekOffsetStruct pos(eSelectEndLine, eDirNext, 0,
nsPoint(0, 0), false, true, false,
true);
true, false);
if (NS_SUCCEEDED(frameBefore->PeekOffset(&pos))) {
theFrame = pos.mResultFrame;
theFrameOffset = pos.mContentOffset;

View File

@ -0,0 +1,21 @@
<html class="reftest-wait">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
</head>
<body onload="start()">
<div onfocus="done()" contenteditable>foo<img alt="IMAGE">bar</div>
<script>
var div = document.querySelector("div");
function start() {
div.focus();
}
function done() {
var sel = getSelection();
// Set the caret right before "bar"
sel.collapse(div.lastChild, 0);
document.documentElement.removeAttribute("class");
}
</script>
</body>
</html>

View File

@ -0,0 +1,24 @@
<html class="reftest-wait">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
</head>
<body onload="start()">
<div onfocus="done()" contenteditable>foo<img alt="IMAGE">bar</div>
<script>
var div = document.querySelector("div");
function start() {
div.focus();
}
function done() {
var sel = getSelection();
sel.collapse(div, 0);
// Press Right four times to set the caret right before "bar"
for (var i = 0; i < 4; ++i) {
synthesizeKey("VK_RIGHT", {});
}
document.documentElement.removeAttribute("class");
}
</script>
</body>
</html>

View File

@ -0,0 +1,26 @@
<html class="reftest-wait">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
<style>
span:before {
content: "IMAGE";
}
</style>
</head>
<body onload="start()">
<div onfocus="done()" contenteditable>foo<span></span>bar</div>
<script>
var div = document.querySelector("div");
function start() {
div.focus();
}
function done() {
var sel = getSelection();
// Set the caret right before "bar"
sel.collapse(div.lastChild, 0);
document.documentElement.removeAttribute("class");
}
</script>
</body>
</html>

View File

@ -0,0 +1,29 @@
<html class="reftest-wait">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
<style>
span:before {
content: "IMAGE";
}
</style>
</head>
<body onload="start()">
<div onfocus="done()" contenteditable>foo<span></span>bar</div>
<script>
var div = document.querySelector("div");
function start() {
div.focus();
}
function done() {
var sel = getSelection();
sel.collapse(div, 0);
// Press Right four times to set the caret right before "bar"
for (var i = 0; i < 4; ++i) {
synthesizeKey("VK_RIGHT", {});
}
document.documentElement.removeAttribute("class");
}
</script>
</body>
</html>

View File

@ -53,6 +53,10 @@ support-files =
bug570378-persian-5.html
bug570378-persian-5-ref.html
bug644768.html
bug989012-1.html
bug989012-1-ref.html
bug989012-2.html
bug989012-2-ref.html
bug1061468.html
bug1061468-ref.html
bug1109968-1-ref.html

View File

@ -107,6 +107,8 @@ var tests = [
[ 'bug106855-2.html' , 'bug106855-1-ref.html' ] ,
[ 'bug389321-2.html' , 'bug389321-2-ref.html' ] ,
[ 'bug613807-1.html' , 'bug613807-1-ref.html' ] ,
[ 'bug989012-1.html' , 'bug989012-1-ref.html' ] ,
[ 'bug989012-2.html' , 'bug989012-2-ref.html' ] ,
[ 'bug1082486-1.html', 'bug1082486-1-ref.html'] ,
[ 'bug1082486-2.html', 'bug1082486-2-ref.html'] ,
// The following test cases are all involving with one sending

View File

@ -3099,6 +3099,7 @@ nsFrame::PeekBackwardAndForward(nsSelectionAmount aAmountBack,
aJumpLines,
true, //limit on scrolled views
false,
false,
false);
rv = PeekOffset(&pos);
if (NS_SUCCEEDED(rv)) {
@ -3115,6 +3116,7 @@ nsFrame::PeekBackwardAndForward(nsSelectionAmount aAmountBack,
aJumpLines,
true, //limit on scrolled views
false,
false,
false);
rv = baseFrame->PeekOffset(&startpos);
if (NS_FAILED(rv))
@ -3127,6 +3129,7 @@ nsFrame::PeekBackwardAndForward(nsSelectionAmount aAmountBack,
aJumpLines,
true, //limit on scrolled views
false,
false,
false);
rv = PeekOffset(&endpos);
if (NS_FAILED(rv))
@ -6491,10 +6494,12 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos)
movedOverNonSelectableText |= (peekSearchState == CONTINUE_UNSELECTABLE);
if (peekSearchState != FOUND) {
bool movedOverNonSelectable = false;
result =
current->GetFrameFromDirection(aPos->mDirection, aPos->mVisual,
aPos->mJumpLines, aPos->mScrollViewStop,
&current, &offset, &jumpedLine);
&current, &offset, &jumpedLine,
&movedOverNonSelectable);
if (NS_FAILED(result))
return result;
@ -6502,11 +6507,18 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos)
// to eat non-renderable content on the new line.
if (jumpedLine)
eatingNonRenderableWS = true;
// Remember if we moved over non-selectable text when finding another frame.
if (movedOverNonSelectable) {
movedOverNonSelectableText = true;
}
}
// Found frame, but because we moved over non selectable text we want the offset
// to be at the frame edge.
if (peekSearchState == FOUND && movedOverNonSelectableText)
// to be at the frame edge. Note that if we are extending the selection, this
// doesn't matter.
if (peekSearchState == FOUND && movedOverNonSelectableText &&
!aPos->mExtend)
{
int32_t start, end;
current->GetOffsets(start, end);
@ -6584,11 +6596,12 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos)
if (!done) {
nsIFrame* nextFrame;
int32_t nextFrameOffset;
bool jumpedLine;
bool jumpedLine, movedOverNonSelectableText;
result =
current->GetFrameFromDirection(aPos->mDirection, aPos->mVisual,
aPos->mJumpLines, aPos->mScrollViewStop,
&nextFrame, &nextFrameOffset, &jumpedLine);
&nextFrame, &nextFrameOffset, &jumpedLine,
&movedOverNonSelectableText);
// We can't jump lines if we're looking for whitespace following
// non-whitespace, and we already encountered non-whitespace.
if (NS_FAILED(result) ||
@ -6936,8 +6949,9 @@ nsFrame::GetLineNumber(nsIFrame *aFrame, bool aLockScroll, nsIFrame** aContainin
nsresult
nsIFrame::GetFrameFromDirection(nsDirection aDirection, bool aVisual,
bool aJumpLines, bool aScrollViewStop,
nsIFrame** aOutFrame, int32_t* aOutOffset, bool* aOutJumpedLine)
bool aJumpLines, bool aScrollViewStop,
nsIFrame** aOutFrame, int32_t* aOutOffset,
bool* aOutJumpedLine, bool* aOutMovedOverNonSelectableText)
{
nsresult result;
@ -6948,6 +6962,7 @@ nsIFrame::GetFrameFromDirection(nsDirection aDirection, bool aVisual,
*aOutFrame = nullptr;
*aOutOffset = 0;
*aOutJumpedLine = false;
*aOutMovedOverNonSelectableText = false;
// Find the prev/next selectable frame
bool selectable = false;
@ -7037,6 +7052,9 @@ nsIFrame::GetFrameFromDirection(nsDirection aDirection, bool aVisual,
return NS_ERROR_FAILURE;
traversedFrame->IsSelectable(&selectable, nullptr);
if (!selectable) {
*aOutMovedOverNonSelectableText = true;
}
} // while (!selectable)
*aOutOffset = (aDirection == eDirNext) ? 0 : -1;

View File

@ -68,6 +68,7 @@ struct MOZ_STACK_CLASS nsPeekOffsetStruct
bool aScrollViewStop,
bool aIsKeyboardSelect,
bool aVisual,
bool aExtend,
mozilla::EWordMovementType aWordMovementType = mozilla::eDefaultBehavior);
// Note: Most arguments (input and output) are only used with certain values
@ -123,6 +124,9 @@ struct MOZ_STACK_CLASS nsPeekOffsetStruct
// Used with: eSelectCharacter, eSelectWord, eSelectBeginLine, eSelectEndLine.
bool mVisual;
// mExtend: Whether the selection is being extended or moved.
bool mExtend;
/*** Output arguments ***/
// mResultContent: Content reached as a result of the peek.

View File

@ -2450,10 +2450,13 @@ public:
* @param aOutOffset [out] 0 indicates that we arrived at the beginning of the output frame;
* -1 indicates that we arrived at its end.
* @param aOutJumpedLine [out] whether this frame and the returned frame are on different lines
* @param aOutMovedOverNonSelectableText [out] whether we jumped over a non-selectable
* frame during the search
*/
nsresult GetFrameFromDirection(nsDirection aDirection, bool aVisual,
bool aJumpLines, bool aScrollViewStop,
nsIFrame** aOutFrame, int32_t* aOutOffset, bool* aOutJumpedLine);
bool aJumpLines, bool aScrollViewStop,
nsIFrame** aOutFrame, int32_t* aOutOffset,
bool* aOutJumpedLine, bool* aOutMovedOverNonSelectableText);
/**
* called to see if the children of the frame are visible from indexstart to index end.

View File

@ -112,6 +112,7 @@ nsPeekOffsetStruct::nsPeekOffsetStruct(nsSelectionAmount aAmount,
bool aScrollViewStop,
bool aIsKeyboardSelect,
bool aVisual,
bool aExtend,
EWordMovementType aWordMovementType)
: mAmount(aAmount)
, mDirection(aDirection)
@ -122,6 +123,7 @@ nsPeekOffsetStruct::nsPeekOffsetStruct(nsSelectionAmount aAmount,
, mScrollViewStop(aScrollViewStop)
, mIsKeyboardSelect(aIsKeyboardSelect)
, mVisual(aVisual)
, mExtend(aExtend)
, mResultContent()
, mResultFrame(nullptr)
, mContentOffset(0)
@ -852,7 +854,8 @@ nsFrameSelection::MoveCaret(nsDirection aDirection,
//set data using mLimiter to stop on scroll views. If we have a limiter then we stop peeking
//when we hit scrollable views. If no limiter then just let it go ahead
nsPeekOffsetStruct pos(aAmount, eDirPrevious, offsetused, desiredPos,
true, mLimiter != nullptr, true, visualMovement);
true, mLimiter != nullptr, true, visualMovement,
aContinueSelection);
nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(frame);
@ -1139,10 +1142,11 @@ nsFrameSelection::GetPrevNextBidiLevels(nsIContent* aNode,
nsIFrame *newFrame;
int32_t offset;
bool jumpedLine;
bool jumpedLine, movedOverNonSelectableText;
nsresult rv = currentFrame->GetFrameFromDirection(direction, false,
aJumpLines, true,
&newFrame, &offset, &jumpedLine);
&newFrame, &offset, &jumpedLine,
&movedOverNonSelectableText);
if (NS_FAILED(rv))
newFrame = nullptr;
@ -1444,7 +1448,7 @@ nsFrameSelection::HandleDrag(nsIFrame *aFrame, nsPoint aPoint)
// first move one character forward.
nsPeekOffsetStruct charPos(eSelectCharacter, eDirNext, offset,
nsPoint(0, 0), false, mLimiter != nullptr,
false, false);
false, false, false);
if (NS_SUCCEEDED(frame->PeekOffset(&charPos))) {
frame = charPos.mResultFrame;
offset = charPos.mContentOffset;
@ -1452,7 +1456,7 @@ nsFrameSelection::HandleDrag(nsIFrame *aFrame, nsPoint aPoint)
}
nsPeekOffsetStruct pos(amount, direction, offset, nsPoint(0, 0),
false, mLimiter != nullptr, false, false);
false, mLimiter != nullptr, false, false, false);
if (frame && NS_SUCCEEDED(frame->PeekOffset(&pos)) && pos.mResultContent) {
offsets.content = pos.mResultContent;