Bug 211164 – Web Inspector: don't immediately attempt to perform a search whenever the find string changes

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

# STEPS TO REPRODUCE
1. inspect any page
2. go to the Console Tab
3. highlight `foo` and press ⌘E
4. focus Web Inspector 
 => the filter in the split console changes to show `foo`

We shouldn't change the filter until the user presses ⌘G or ⇧⌘G.

[Devin Rousso]

Created attachment 397928 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 397928 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:16
> +        (WI.LogContentView.prototype.handleFindStringUpdated): Deleted.
> +        Only update the `searchQuery` of the `WI.FindBanner` and `performSearch` when the find
> +        next/previous keyboard shortcut is activated, not when the find string is populated/updated.

This doesn't sound like it matches macOS System behavior.

For example, in a Safari Tab.
1. Show Find Banner (⌘F)
2. Type some text in there "foo"
3. Highlight some text on the page (<-- like this line)
4. Populate Search Pasteboard (⌘E)
  => Immediately the find banner contents update, a search is not yet performed but the contents have updated.

[Devin Rousso]

Comment on attachment 397928 [details]
Patch

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

>> Source/WebInspectorUI/ChangeLog:16
>> +        next/previous keyboard shortcut is activated, not when the find string is populated/updated.
> 
> This doesn't sound like it matches macOS System behavior.
> 
> For example, in a Safari Tab.
> 1. Show Find Banner (⌘F)
> 2. Type some text in there "foo"
> 3. Highlight some text on the page (<-- like this line)
> 4. Populate Search Pasteboard (⌘E)
>   => Immediately the find banner contents update, a search is not yet performed but the contents have updated.

As we discussed offline, this is not really possible in Web Inspector because we use an `<input type="search">` which automatically fires a `"search"` event after 1s (or something like that), meaning that if we update the value we will cause a new search to happen.  As you suggested offline, we could change the `type` of the `<input>` to `"text"`, change the `value`, and then change the `type` back to `search`, but that would mean that we now have to maintain a state of "the search query has been updated but we haven't searched with the new string yet", which might not be that easy.  Furthermore, Web Inspector has a different find UI than Safari (a dark layer with holes) or Xcode (bounce the next result once) in that we persist a highlight of all matches (even after interacting with the content view), which I believe could lead to a confusing scenario where the things that are highlighted don't match what's in the find banner.

With that having been said, however, this patch does have one problem in that it doesn't preform a search when ⌘E is pressed, meaning if the find banner is showing, the match count won't update, which is a problem.

[Devin Rousso]

Created attachment 397936 [details]
Patch

[EWS]

Committed r260895: <https://trac.webkit.org/changeset/260895>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397936 [details].

[Radar WebKit Bug Importer]

<rdar://problem/62599550>