1943846 - In-page search may incorrectly behave for characters with Unicode code points greater than U+10000

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Tan Junjian]

Attachment: Screenshot

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:134.0) Gecko/20100101 Firefox/134.0

Steps to reproduce:

Search a character in page of which the Unicode point is U+x00yy where:

• x >= 1
• yy is 09, 0A, 0D or 20.

Here are some of the characters may be used:
[𐀉] [𠀍] [􀀉] [𰀠]

Actual results:

All spaces in page will be matched. These characters also will match each other.

Tested on Linux Firefox 134, 136-nightly, and Windows Firefox.

Expected results:

Only these specific characters will be matched.

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Tan Junjian]

I'm trying to fix it! If anyone has progress please share it here!

Comment 3

[Tan Junjian]

I think I've fixed it! Could anyone kindly assign it to me and review my patch?

Comment 4

[Tom S. (please needinfo tschuster)]

You will automatically be assigned to the bug after submitting the patch.

Comment 5

[Tan Junjian]

Attachment: Bug 1943846 - Fix nsFind::IsSpace to handle UTF-32 characters correctly. r=emilio

The original IsSpace macro uses nsCRT::IsAsciiSpace which doesn't handle
UTF-32 characters (characters above U+FFFF) correctly. Add a check for UTF-32
characters to ensure correct matching behavior. Also add a test case to
test_nsFind.html to verify the fix for Bug 1943846.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1882c174191
Fix nsFind::IsSpace to handle UTF-32 characters correctly. r=emilio

Comment 7

[Atila Butkovits]

Backed out for causing bustages at nsFind.cpp.

Backout link: https://hg.mozilla.org/integration/autoland/rev/441b1202a649de00c6ad0c83a9083003550d4bf3

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e1882c174191a6cc1fb9d2dadcbb1093de5b0fb2

Failure log: https://treeherder.mozilla.org/logviewer?job_id=492837035&repo=autoland&lineNumber=45250

Comment 8

[Tan Junjian]

(In reply to Atila Butkovits from comment #7)

Backed out for causing bustages at nsFind.cpp.

Backout link: https://hg.mozilla.org/integration/autoland/rev/441b1202a649de00c6ad0c83a9083003550d4bf3

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e1882c174191a6cc1fb9d2dadcbb1093de5b0fb2

Failure log: https://treeherder.mozilla.org/logviewer?job_id=492837035&repo=autoland&lineNumber=45250

Sorry for the inconvenient. I updated the patch.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Ni?ing to run this through all tests.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b71c8c052463
Fix nsFind::IsSpace to handle UTF-32 characters correctly. r=emilio

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc959685b9d9
Backed out changeset b71c8c052463 for causing mass failures. CLOSED TREE

Comment 12

[agoloman]

Backed out for causing android mass failures.

• Backout link
• Push with failures
• Failure Log

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Wat

Comment 14

[Tan Junjian]

I'm sorry but I have no idea about this error... I think I need help.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Yeah, can look into it... Maybe some code is choking on the characters on the test or something? But shouldn't really be the case.

Comment 16

[Tan Junjian]

Can it be an occasion? I downloaded the target.crashreporter-symbols.zip and it validates well...

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Yeah I suspect it was some infra fluke. I pushed this to try to check tho. https://treeherder.mozilla.org/jobs?repo=try&revision=733705b4546b315b4059396fcc0c12ff411b7e7f

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Yeah seems good now, will re-queue.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/832cb4dc696a
Fix nsFind::IsSpace to handle UTF-32 characters correctly. r=emilio

Comment 20

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/832cb4dc696a