nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1612076 - Make AdjustCaretFrameForLineEnd stop scanning text node if first node is editable but reached non-editable one r=emilio

Browse source 
When pressing `ArrowLeft` key at:
```
<p>abc</p><p><span contenteditable=false>def</span>{}<br></p>
```
, caret is moved into the non-editable text because
`nsCaret::GetCaretFrameForNodeOffset` looks for a preceding text node from
the `BRFrame`. This is required if the preceding text node ends with a
collapsible white-space but followed by a `<br>` because the text frame
should not contain the white-space rect and `BRFrame` frame should be next
to it, i.e., the white-space looks like a overflown content.

So, for rendering a caret, the method needs to return non-editable text frame
even in the case, but for considering new caret position in the DOM tree, it
should not return non-editable text frame.

Therefore, this patch adds a param to `nsCaret::GetCaretFrameForNodeOffset()`
which forcibly return editable content frame or not and makes
`Selection::GetPrimaryOrCaretFrameForNodeOffset` call it with the new option
because its callers are the handler of caret navigation.

Differential Revision: https://phabricator.services.mozilla.com/D196259
This commit is contained in:
Masayuki Nakano 2023-12-19 01:58:07 +00:00
parent 38db0ec252
commit 87da8e59cd
5 changed files with 235 additions and 18 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

2
dom/base/Selection.cpp
View file

@ -1599,6 +1599,8 @@ nsIFrame* Selection::GetPrimaryOrCaretFrameForNodeOffset(nsIContent* aContent,
return nsCaret::GetCaretFrameForNodeOffset(
mFrameSelection, aContent, aOffset, hint, caretBidiLevel,
aContent && aContent->IsEditable() ? nsCaret::ForceEditableRegion::Yes
: nsCaret::ForceEditableRegion::No,
/* aReturnUnadjustedFrame = */ nullptr, aOffsetUsed);
}

48
layout/base/nsCaret.cpp
View file

@ -66,8 +66,9 @@ static nsIFrame* CheckForTrailingTextFrameRecursive(nsIFrame* aFrame,
}
for (nsIFrame* f : aFrame->PrincipalChildList()) {
nsIFrame* r = CheckForTrailingTextFrameRecursive(f, aStopAtFrame);
if (r) return r;
if (nsIFrame* r = CheckForTrailingTextFrameRecursive(f, aStopAtFrame)) {
return r;
}
}
return nullptr;
}
@ -86,7 +87,8 @@ static nsLineBox* FindContainingLine(nsIFrame* aFrame) {
return nullptr;
}
static void AdjustCaretFrameForLineEnd(nsIFrame** aFrame, int32_t* aOffset) {
static void AdjustCaretFrameForLineEnd(nsIFrame** aFrame, int32_t* aOffset,
bool aEditableOnly) {
nsLineBox* line = FindContainingLine(*aFrame);
if (!line) {
return;
@ -98,14 +100,21 @@ static void AdjustCaretFrameForLineEnd(nsIFrame** aFrame, int32_t* aOffset) {
if (r == *aFrame) {
return;
}
if (r) {
// We found our frame, but we may not be able to properly paint the caret
// if -moz-user-modify differs from our actual frame.
MOZ_ASSERT(r->IsTextFrame(), "Expected text frame");
*aFrame = r;
*aOffset = (static_cast<nsTextFrame*>(r))->GetContentEnd();
if (!r) {
continue;
}
// If found text frame is non-editable but the start frame content is
// editable, we don't want to put caret into the non-editable text node.
// We should return the given frame as-is in this case.
if (aEditableOnly && !r->GetContent()->IsEditable()) {
return;
}
// We found our frame, but we may not be able to properly paint the caret
// if -moz-user-modify differs from our actual frame.
MOZ_ASSERT(r->IsTextFrame(), "Expected text frame");
*aFrame = r;
*aOffset = (static_cast<nsTextFrame*>(r))->GetContentEnd();
return;
}
}
@ -424,7 +433,7 @@ nsIFrame* nsCaret::GetFrameAndOffset(const Selection* aSelection,
return nsCaret::GetCaretFrameForNodeOffset(
frameSelection, contentNode, focusOffset, frameSelection->GetHint(),
bidiLevel, aUnadjustedFrame, aFrameOffset);
bidiLevel, ForceEditableRegion::No, aUnadjustedFrame, aFrameOffset);
}
/* static */
@ -681,13 +690,11 @@ void nsCaret::StopBlinking() {
}
}
nsIFrame* nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection,
nsIContent* aContentNode,
int32_t aOffset,
CaretAssociationHint aFrameHint,
BidiEmbeddingLevel aBidiLevel,
nsIFrame** aReturnUnadjustedFrame,
int32_t* aReturnOffset) {
nsIFrame* nsCaret::GetCaretFrameForNodeOffset(
nsFrameSelection* aFrameSelection, nsIContent* aContentNode,
int32_t aOffset, CaretAssociationHint aFrameHint,
BidiEmbeddingLevel aBidiLevel, ForceEditableRegion aForceEditableRegion,
nsIFrame** aReturnUnadjustedFrame, int32_t* aReturnOffset) {
if (!aFrameSelection) {
return nullptr;
}
@ -702,6 +709,9 @@ nsIFrame* nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection,
return nullptr;
}
MOZ_ASSERT_IF(aForceEditableRegion == ForceEditableRegion::Yes,
aContentNode->IsEditable());
nsIFrame* theFrame = nullptr;
int32_t theFrameOffset = 0;
@ -722,7 +732,9 @@ nsIFrame* nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection,
// (e.g. if theFrame is a <br> frame), then put the caret at the end of
// that text frame instead. This way, the caret will be positioned as if
// trailing whitespace was not trimmed.
AdjustCaretFrameForLineEnd(&theFrame, &theFrameOffset);
AdjustCaretFrameForLineEnd(
&theFrame, &theFrameOffset,
aForceEditableRegion == ForceEditableRegion::Yes);
}
// Mamdouh : modification of the caret to work at rtl and ltr with Bidi

2
layout/base/nsCaret.h
View file

@ -178,10 +178,12 @@ class nsCaret final : public nsISelectionListener {
*/
static nsIFrame* GetGeometry(const mozilla::dom::Selection* aSelection,
nsRect* aRect);
enum class ForceEditableRegion { No, Yes };
static nsIFrame* GetCaretFrameForNodeOffset(
nsFrameSelection* aFrameSelection, nsIContent* aContentNode,
int32_t aOffset, CaretAssociationHint aFrameHint,
mozilla::intl::BidiEmbeddingLevel aBidiLevel,
ForceEditableRegion aForceEditableRegion,
nsIFrame** aReturnUnadjustedFrame, int32_t* aReturnOffset);
static nsRect GetGeometryForFrame(nsIFrame* aFrame, int32_t aFrameOffset,
nscoord* aBidiIndicatorSize);

12
testing/web-platform/meta/selection/caret/move-around-contenteditable-false.html.ini Normal file
View file

@ -0,0 +1,12 @@
[move-around-contenteditable-false.html]
[Move caret from end of editable text node to <br> following non-editable text in next paragraph: first arrow-right should move caret before non-editable text]
expected: FAIL
[Move caret from <br> following non-editable text to end of preceding editable text in next paragraph: first arrow-left should move caret before non-editable text]
expected: FAIL
[Move caret from empty editable paragraph to editable text following non-editable text in next paragraph: first arrow-right should move caret before non-editable text]
expected: FAIL
[Move caret from empty editable paragraph to editable text following non-editable text in next paragraph: second arrow-right should move caret after non-editable text]
expected: FAIL

189
testing/web-platform/tests/selection/caret/move-around-contenteditable-false.html Normal file
View file

@ -0,0 +1,189 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Don't move caret to non-editable node from a editable node</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script>
"use strict";
function getRangeDescription(range) {
function getNodeDescription(node) {
if (!node) {
return "null";
}
switch (node.nodeType) {
case Node.TEXT_NODE:
return `${node.nodeName} "${node.data}"`;
case Node.ELEMENT_NODE:
return `<${node.nodeName.toLowerCase()}>`;
default:
return `${node.nodeName}`;
}
}
if (range === null) {
return "null";
}
if (range === undefined) {
return "undefined";
}
return range.startContainer == range.endContainer &&
range.startOffset == range.endOffset
? `(${getNodeDescription(range.startContainer)}, ${range.startOffset})`
: `(${getNodeDescription(range.startContainer)}, ${
range.startOffset
}) - (${getNodeDescription(range.endContainer)}, ${range.endOffset})`;
}
function sendArrowRightKey() {
const kArrowRight = "\uE014";
return new test_driver.Actions()
.keyDown(kArrowRight)
.keyUp(kArrowRight)
.send();
}
function sendArrowLeftKey() {
const kArrowLeft = "\uE012";
return new test_driver.Actions()
.keyDown(kArrowLeft)
.keyUp(kArrowLeft)
.send();
}
promise_test(async () => {
await new Promise(resolve => {
addEventListener("load", resolve, {once: true});
});
}, "Initializing tests");
promise_test(async t => {
const editingHost = document.querySelector("div[contenteditable]");
editingHost.focus();
const p = editingHost.querySelector("p");
getSelection().collapse(p.firstChild, "abc".length);
await sendArrowRightKey();
test(() => {
assert_equals(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: p.nextSibling,
startOffset: 0,
endContainer: p.nextSibling,
endOffset: 0,
}),
);
}, `${t.name}: first arrow-right should move caret before non-editable text`);
await sendArrowRightKey();
test(() => {
assert_equals(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: p.nextSibling,
startOffset: 1,
endContainer: p.nextSibling,
endOffset: 1,
}),
);
}, `${t.name}: second arrow-right should move caret after non-editable text`);
}, "Move caret from end of editable text node to <br> following non-editable text in next paragraph");
promise_test(async t => {
const editingHost = document.querySelector("div[contenteditable]");
editingHost.focus();
const p = editingHost.querySelector("p");
getSelection().collapse(p.nextSibling, 1);
await sendArrowLeftKey();
assert_false(
editingHost.querySelector("[contenteditable=false]").contains(getSelection().focusNode),
"focus node should not be the non-editable nodes"
);
assert_false(
editingHost.querySelector("[contenteditable=false]").contains(getSelection().anchorNode),
"anchor node should not be the non-editable nodes"
);
test(() => {
assert_equals(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: p.nextSibling,
startOffset: 0,
endContainer: p.nextSibling,
endOffset: 0,
}),
);
}, `${t.name}: first arrow-left should move caret before non-editable text`);
}, "Move caret from <br> following non-editable text to end of preceding editable text in next paragraph");
promise_test(async t => {
const editingHost = document.querySelector("div[contenteditable] + div[contenteditable]");
editingHost.focus();
const p = editingHost.querySelector("p");
getSelection().collapse(p.firstChild, 0);
await sendArrowRightKey();
test(() => {
assert_equals(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: p.nextSibling,
startOffset: 0,
endContainer: p.nextSibling,
endOffset: 0,
}),
);
}, `${t.name}: first arrow-right should move caret before non-editable text`);
await sendArrowRightKey();
test(() => {
assert_equals(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost.querySelector("[contenteditable=false]").nextSibling,
startOffset: 0,
endContainer: editingHost.querySelector("[contenteditable=false]").nextSibling,
endOffset: 0,
}),
);
}, `${t.name}: second arrow-right should move caret after non-editable text`);
}, "Move caret from empty editable paragraph to editable text following non-editable text in next paragraph");
promise_test(async t => {
const editingHost = document.querySelector("div[contenteditable] + div[contenteditable]");
editingHost.focus();
const p = editingHost.querySelector("p");
getSelection().collapse(editingHost.querySelector("[contenteditable=false]").nextSibling, 0);
await sendArrowLeftKey();
assert_false(
editingHost.querySelector("[contenteditable=false]").contains(getSelection().focusNode),
"focus node should not be the non-editable nodes"
);
assert_false(
editingHost.querySelector("[contenteditable=false]").contains(getSelection().anchorNode),
"anchor node should not be the non-editable nodes"
);
test(() => {
assert_equals(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: p.nextSibling,
startOffset: 0,
endContainer: p.nextSibling,
endOffset: 0,
}),
);
}, `${t.name}: first arrow-left should move caret before non-editable text`);
}, "Move caret from start of text following non-editable text to empty preceding editable paragraph");
</script>
</head>
<body>
<div contenteditable>
<p>abc</p><p><span contenteditable="false">def</span><br></p>
</div>
<div contenteditable>
<p><br></p><p><span contenteditable="false">abc</span>def</p>
</div>
</body>
</html>