Bug 111472 – Chromium - Find field display negative indexes

• Component: WebKit API
• Status: RESOLVED WONTFIX

[Sam Larison]

The chrome bug found at https://code.google.com/p/chromium/issues/detail?id=130908

Seems to be related to problems in code that is located in the file:

src\third_party\WebKit\Source\WebKit\chromium\src\WebFrameImpl.cpp
aka
Source\WebKit\chromium\src\WebFrameImpl.cpp

A simple fix as shown in comment number 9 has been discovered
https://code.google.com/p/chromium/issues/detail?id=130908#c9

It is unclear to me whether this fix will be considered sufficient or a more intensive overhaul will be required.

I did not see a category for "WebKit chromium" in the component selector...

I have a branch in webkit git with above fix included.

[Sam Larison]

Created attachment 191560 [details]
A proposed fix for chromium issue 130908

[Sam Larison]

Created attachment 191597 [details]
A proposed fix for chromium issue 130908

[Sam Larison]

Comment on attachment 191597 [details]
A proposed fix for chromium issue 130908

The comment says "we need to reset everything" yet everything was not being reset... until now.

[Sam Larison]

Adding some people to the CC list who may have more familiarity with this code to aid in resolution of this bug.
https://code.google.com/p/chromium/issues/detail?id=130908

The line added by this patch fixes the chrome issue.  I am looking for some additional information from those who originally worked on this bit of code.  Maybe there is a good reason this variable is not reset during rest.

Unless m_locatingActiveRect becomes true we do not discover the correct index and end up with negative numbers in some circumstances as seen in the chrome issue.  When setting the reset parameter to true for scopeStringMatches, the function is not truly reset unless m_locatingActiveRect becomes true.

I can see there may be other ways to resolve the issue (calling the find function with certain parameters sets this variable to become true), however perhaps I might find a better understanding of the issue if I knew why m_locatingActiveRect isn't being reset during reset (speed?), because at least under some circumstances it seems like it should be set to true too, otherwise critical parts of scopeStringMatches are never reached.

I am running more tests to see if this resolves too.
https://code.google.com/p/chromium/issues/detail?id=162083

Thanks!
Sam

[Adam Barth]

Comment on attachment 191597 [details]
A proposed fix for chromium issue 130908

We'll need a change log and a test as well.  I haven't studied the change to see if its right.

[Leandro Graciá Gil]

I'm afraid I'm not familiar with m_locatingActiveRect nor I touched it when working on the WebFrameImpl find-in-page code. Sorry for not being of more help.

[Sam Larison]

Created attachment 193673 [details]
Patch