зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1601118 - Fix nsFind state initialization logic. r=smaug
We should start seeking from the child-at-start/end-offset, rather than the selection container. The logic to select the starting node was backwards too... If we're finding backwards we want to start looking from the beginning of the range and vice versa. But I think that doesn't really matter since nsWebBrowserFind always gives us a start range with start == end. Differential Revision: https://phabricator.services.mozilla.com/D63553 --HG-- extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez
Родитель
8486885f18
Коммит
7ee58cb469
|
|
@ -662,6 +662,8 @@ skip-if = toolkit == 'android' && !is_fennec # Bug 1525959
|
|||
skip-if = (toolkit == 'android') # Android: Bug 1465387
|
||||
[test_find_nac.html]
|
||||
skip-if = (toolkit == 'android') # Android: Bug 1465387
|
||||
[test_find_bug1601118.html]
|
||||
skip-if = (toolkit == 'android') # Android: Bug 1465387
|
||||
[test_focus_shadow_dom_root.html]
|
||||
[test_focus_shadow_dom.html]
|
||||
[test_getAttribute_after_createAttribute.html]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,61 @@
|
|||
<!doctype html>
|
||||
<meta charset="utf-8">
|
||||
<script src="/resources/testharness.js"></script>
|
||||
<script src="/resources/testharnessreport.js"></script>
|
||||
<div id="container">
|
||||
Fission <br/>
|
||||
Some text <br/>
|
||||
Some more text Fission or Fission or even <span>Fi</span>ssion<br/>
|
||||
Fission <br/>
|
||||
more text<br/>
|
||||
<div>
|
||||
in a nested block Fission stuff
|
||||
</div>
|
||||
</div>
|
||||
<script>
|
||||
const kContainer = document.getElementById("container");
|
||||
const kExpectedCount = 6;
|
||||
|
||||
// We expect surroundContents() to throw in the <span>Fi</span>ssion case.
|
||||
const kExpectedThrewCount = 1;
|
||||
|
||||
// Keep a hang of the original DOM so as to test forwards and backwards navigation.
|
||||
const kContainerClone = kContainer.cloneNode(true);
|
||||
|
||||
function advance(backwards) {
|
||||
if (!window.find("Fiss", /* caseSensitive = */ true, backwards))
|
||||
return { success: false };
|
||||
|
||||
let threw = false;
|
||||
try {
|
||||
window.getSelection().getRangeAt(0).surroundContents(document.createElement("mark"));
|
||||
} catch (ex) {
|
||||
threw = true;
|
||||
}
|
||||
|
||||
// Sanity-check
|
||||
assert_equals(window.getSelection().toString(), "Fiss");
|
||||
return { success: true, threw };
|
||||
}
|
||||
|
||||
function runTestForDirection(backwards) {
|
||||
let threwCount = 0;
|
||||
for (let i = 0; i < kExpectedCount; ++i) {
|
||||
let result = advance(backwards);
|
||||
assert_true(result.success, `Should've successfully advanced (${i} / ${kExpectedCount}, backwards: ${backwards})`);
|
||||
if (result.threw)
|
||||
threwCount++;
|
||||
}
|
||||
assert_false(advance(backwards).success, `Should've had exactly ${kExpectedCount} matches`);
|
||||
assert_equals(threwCount, kExpectedThrewCount, "Should've thrown the expected number of times");
|
||||
assert_equals(kContainer.querySelectorAll("mark").length, kExpectedCount - threwCount, "Should've created the expected number of marks");
|
||||
assert_false(!!kContainer.querySelector("mark mark"), "Shouldn't have created nested marks");
|
||||
}
|
||||
|
||||
test(function() {
|
||||
runTestForDirection(false);
|
||||
window.getSelection().removeAllRanges();
|
||||
kContainer.innerHTML = kContainerClone.innerHTML;
|
||||
runTestForDirection(true);
|
||||
}, "window.find() while marking with surroundContents");
|
||||
</script>
|
||||
|
|
@ -243,6 +243,10 @@ struct nsFind::State final {
|
|||
// Sets up the first node position and offset.
|
||||
void Initialize();
|
||||
|
||||
static bool ValidTextNode(const nsINode& aNode) {
|
||||
return aNode.IsText() && !SkipNode(aNode.AsText());
|
||||
}
|
||||
|
||||
const bool mFindBackward;
|
||||
|
||||
// Whether we've called GetNextNode() at least once.
|
||||
|
|
@ -265,15 +269,7 @@ void nsFind::State::Advance() {
|
|||
nsIContent* current =
|
||||
mFindBackward ? mIterator.GetPrev() : mIterator.GetNext();
|
||||
|
||||
if (!current) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!current->IsContent() || SkipNode(current->AsContent())) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (current->IsText()) {
|
||||
if (!current || ValidTextNode(*current)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
|
@ -284,11 +280,23 @@ void nsFind::State::Initialize() {
|
|||
mInitialized = true;
|
||||
mIterOffset = mFindBackward ? -1 : 0;
|
||||
|
||||
nsINode* container = mFindBackward ? mStartPoint.GetStartContainer()
|
||||
: mStartPoint.GetEndContainer();
|
||||
|
||||
// Set up ourselves at the first node we want to start searching at.
|
||||
nsINode* beginning = mFindBackward ? mStartPoint.GetEndContainer()
|
||||
: mStartPoint.GetStartContainer();
|
||||
if (beginning && beginning->IsContent()) {
|
||||
mIterator.Seek(*beginning->AsContent());
|
||||
nsIContent* beginning = mFindBackward ? mStartPoint.GetChildAtStartOffset()
|
||||
: mStartPoint.GetChildAtEndOffset();
|
||||
if (beginning) {
|
||||
mIterator.Seek(*beginning);
|
||||
// If the start point is pointing to a node, when looking backwards we'd
|
||||
// start looking at the children of that node, and we don't really want
|
||||
// that. When looking forwards, we look at the next sibling afterwards.
|
||||
if (mFindBackward) {
|
||||
mIterator.GetPrevSkippingChildren();
|
||||
}
|
||||
} else if (container && container->IsContent()) {
|
||||
// Text-only range, or pointing to past the end of the node, for example.
|
||||
mIterator.Seek(*container->AsContent());
|
||||
}
|
||||
|
||||
nsINode* current = mIterator.GetCurrent();
|
||||
|
|
@ -296,19 +304,22 @@ void nsFind::State::Initialize() {
|
|||
return;
|
||||
}
|
||||
|
||||
if (!current->IsText() || SkipNode(current->AsText())) {
|
||||
if (!ValidTextNode(*current)) {
|
||||
Advance();
|
||||
return;
|
||||
current = mIterator.GetCurrent();
|
||||
if (!current) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
mLastBlockParent = GetBlockParent(*current->AsText());
|
||||
|
||||
if (current != beginning) {
|
||||
if (current != container) {
|
||||
return;
|
||||
}
|
||||
|
||||
mIterOffset =
|
||||
mFindBackward ? mStartPoint.EndOffset() : mStartPoint.StartOffset();
|
||||
mFindBackward ? mStartPoint.StartOffset() : mStartPoint.EndOffset();
|
||||
}
|
||||
|
||||
const nsTextFragment* nsFind::State::GetNextNonEmptyTextFragmentInSameBlock() {
|
||||
|
|
|
|||
|
|
@ -826,6 +826,10 @@ nsresult nsTypeAheadFind::GetSearchContainers(
|
|||
// IsRangeVisible. It returns the first visible range after searchRange
|
||||
IsRangeVisible(mSearchRange, aIsFirstVisiblePreferred, true,
|
||||
getter_AddRefs(mStartPointRange), nullptr);
|
||||
// We want to search in the visible selection range. That means that the
|
||||
// start point needs to be the end if we're looking backwards, or vice
|
||||
// versa.
|
||||
mStartPointRange->Collapse(!aFindPrev);
|
||||
} else {
|
||||
uint32_t startOffset;
|
||||
nsCOMPtr<nsINode> startNode;
|
||||
|
|
@ -844,10 +848,9 @@ nsresult nsTypeAheadFind::GetSearchContainers(
|
|||
// We need to set the start point this way, other methods haven't worked
|
||||
mStartPointRange->SelectNode(*startNode, IgnoreErrors());
|
||||
mStartPointRange->SetStart(*startNode, startOffset, IgnoreErrors());
|
||||
mStartPointRange->Collapse(true); // collapse to start
|
||||
}
|
||||
|
||||
mStartPointRange->Collapse(true); // collapse to start
|
||||
|
||||
presShell.forget(aPresShell);
|
||||
presContext.forget(aPresContext);
|
||||
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче