Bug 30731 – Web Inspector: Clash Between Search's onkeyup and incremental search events

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Joseph Pecoraro]

When quickly forcing a search on a short query with "Enter" (<3 characters) the Inspector's search field fires both its onkeyup and incremental search events, causing conflicts.  The "search" does see the forced search and clobbers the earlier fired forced search.

Example Video. The first search causes a conflict due to "quickly" searching, the second goes through fine because no incremental search event fired due to the text not changing:
http://screencast.com/t/uHozOKDiy7D8

[Joseph Pecoraro]

Created attachment 41759 [details]
[PATCH] Search Fix

[Joseph Pecoraro]

Hmm, the first patch does not handle the following scenario:

1. User quickly searches a 1 character search. No Clobber happens (but the flag is deleted).
2. User types a 2nd character. Clobbering happens.

I guess we should prevent clobbering for any <3 character search if it was forced (meaning keep the flag around).  This complicates things slightly:

Users searches "a", Enter, "b", "c", Backspace
--> "a" does nothing
--> Enter forces search [search "a" happening]
--> "b" does nothing [search "a" happening]
--> "c" spawns incremental search [search "abc" happening]
--> Backspace brings us back into <3 character range, what do we do here?

Current behavior is that backspacing until the query is below 3 characters clears the search. I think we should keep that behavior.

But I do feel that if you forced a search with 1 character then it should be kept (not cleared) at 2 characters.  If this is not desired we can use the first patch.

[Joseph Pecoraro]

Bah. Ignore Comment #2. When a search happens the search text is selected, making my user scenario a highly unlikely scenario.

Instead, I implemented the following behavior: Whenever you make a search, be it short or not... it goes back to normal for the next search.
http://screencast.com/t/IKbwdQWjJ

[Joseph Pecoraro]

Created attachment 41764 [details]
[PATCH] Behavior in Comment #3

[Joseph Pecoraro]

Ugh. The "this.currentQuery !== query", is not necessary and I will remove it before I land if this gets r+.

[Joseph Pecoraro]

The currentQuery comparison was needed for certain cases and was okay to leave in.

Landed in http://trac.webkit.org/changeset/50014
r50014 = 7dabc822eac2979ea589a340500102e389e55e4a