Bug 200589 – Fix Crash in Mail Search

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Fix Crash in Mail Search

[Megan Gardner]

Created attachment 375953 [details]
Patch

[Tim Horton]

Comment on attachment 375953 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375953&action=review

> Source/WebKit/ChangeLog:10
> +        It is possible for AppKit reasons to get a -1 for the index
> +        of the found item. Do not try and insert data in this case.
> +
> +        Reviewed by NOBODY (OOPS!).

Something bad happened here with the order.

Also, it would be nice to be more specific about the "AppKit reasons". Isn't it really about reverse search? (And then, why is it OK not to have any rects? does it actually behave correctly?)

> Source/WebKit/UIProcess/mac/WKTextFinderClient.mm:85
> +            if (matchIndex > 0 && matchIndex < matchCount)
> +                allMatches[matchIndex].appendVector(matchRects);

matchIndex 0 is valid, so you want >=

[Tim Horton]

Please test both Mail and Notes, forward and backward, in conversations and not (in Mail).

[Tim Horton]

ALSO ALSO this should totally be API testable

[Megan Gardner]

Created attachment 375960 [details]
Patch

[Megan Gardner]

Created attachment 375962 [details]
Patch

[Megan Gardner]

Created attachment 375964 [details]
Patch

[Megan Gardner]

Created attachment 375968 [details]
Patch

[Tim Horton]

Comment on attachment 375968 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375968&action=review

> Source/WebKit/UIProcess/mac/WKTextFinderClient.mm:84
> -            allMatches[matchIndex].appendVector(matchRects);
> +            if (matchIndex >= 0 && (uint32_t)matchIndex < matchCount)

This is fine for now given current clients, but maybe leave a FIXME that we should have match rects when doing an incremental backwards search (did you test Notes??).

[Wenson Hsieh]

<rdar://problem/53666720>

[Ryosuke Niwa]

Comment on attachment 375968 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375968&action=review

>> Source/WebKit/UIProcess/mac/WKTextFinderClient.mm:84
>> +            if (matchIndex >= 0 && (uint32_t)matchIndex < matchCount)
> 
> This is fine for now given current clients, but maybe leave a FIXME that we should have match rects when doing an incremental backwards search (did you test Notes??).

Also, please use static_cast<uint32_t>.

[Megan Gardner]

Created attachment 376059 [details]
Patch for landing

[Megan Gardner]

Created attachment 376090 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 376090 [details]
Patch for landing

Clearing flags on attachment: 376090

Committed r248548: <https://trac.webkit.org/changeset/248548>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.