Bug 1704167 - When a pattern ends in whitespace, try to collapse adjacent white-space. r=jfkthame

This I'm not 100% sure. Should be harmless, but it's a bit subtle.
Maybe we should special-case it with "we're at the beginning of the
pattern"? Reasoning below:

The previous patch restores the performance of the original test-case.

However, if you go to the reduced test-case and try to type " re" in the
findbar, we still take a long time. The reason for that is not that the
previous patch is not effective, but that the findbar sends find
requests as soon as you type, and thus we end up with a request to find
" ", which matches a gazillion spaces in the page and causes us to use
tons of memory and time. Finding " re" is actually super-fast :-)

This fixes it, but it is a bit subtle, so thoughts? Perhaps the findbar
should wait a bit to perform the search before sending a query for " "
instead or something? But I'd rather make it fast.

Differential Revision: https://phabricator.services.mozilla.com/D111634
This commit is contained in:
Emilio Cobos Álvarez 2021-04-12 16:11:21 +00:00
Родитель 4437e99191
Коммит 9d38e5590e
2 изменённых файлов: 25 добавлений и 10 удалений

Просмотреть файл

@ -6,7 +6,9 @@
<script>
const t = async_test("Test window.find / nsFind");
function testFindable(isFindable, textToFind, buildDoc, description) {
function testFindable(findCount, textToFind, buildDoc, description) {
if (typeof findCount == "boolean")
findCount = findCount ? 1 : 0;
try {
const iframe = document.querySelector("iframe")
iframe.contentDocument.documentElement.innerHTML =
@ -16,11 +18,15 @@ function testFindable(isFindable, textToFind, buildDoc, description) {
buildDoc(iframe.contentDocument);
iframe.contentWindow.getSelection().removeAllRanges();
assert_equals(
isFindable,
iframe.contentWindow.find(textToFind),
"Should be " + (isFindable ? "" : "not ") + "findable: " + description + ", text: " + textToFind
);
for (let i = findCount; i >= 0; --i) {
const expectFindable = i != 0;
assert_equals(
iframe.contentWindow.find(textToFind),
expectFindable,
"Should be " + (expectFindable ? "" : "not ") + "findable: " + description + ", text: " + textToFind + ", iter: " + (findCount - i + 1)
);
}
} catch (ex) {
assert_unreached(ex);
}
@ -178,6 +184,8 @@ let runTests = t.step_func_done(function() {
testFindable(true, "history.kafka", `
<code>database.history&#8203;.kafka.bootstrap.servers</code>
`, "ZWSP should be ignored");
testFindable(2, " ", "a b c", "Collapsed whitespace");
});
window.onload = function() {

Просмотреть файл

@ -881,16 +881,17 @@ nsFind::Find(const nsAString& aPatText, nsRange* aSearchRange,
// Make the range:
// Check for word break (if necessary)
if (mWordBreaker) {
if (mWordBreaker || inWhitespace) {
int32_t nextfindex = findex + incr;
char16_t nextChar;
// If still in array boundaries, get nextChar.
if (mFindBackward ? (nextfindex >= 0) : (nextfindex < fragLen)) {
if (t2b)
if (t2b) {
nextChar = DecodeChar(t2b, &nextfindex);
else
} else {
nextChar = CHAR_TO_UNICHAR(t1b[nextfindex]);
}
} else {
// Get next character from the next node.
nextChar = PeekNextChar(state, !!matchAnchorNode);
@ -901,10 +902,16 @@ nsFind::Find(const nsAString& aPatText, nsRange* aSearchRange,
}
// If a word break isn't there when it needs to be, reset search.
if (!BreakInBetween(c, nextChar)) {
if (mWordBreaker && !BreakInBetween(c, nextChar)) {
matchAnchorNode = nullptr;
continue;
}
if (inWhitespace && IsSpace(nextChar)) {
// If the next character is also an space, keep going, this space
// will collapse.
continue;
}
}
int32_t matchStartOffset;