Bug 30731 - Web Inspector: Clash Between Search's onkeyup and incremental search events
Summary: Web Inspector: Clash Between Search's onkeyup and incremental search events
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-23 15:58 PDT by Joseph Pecoraro
Modified: 2009-10-23 17:27 PDT (History)
6 users (show)

See Also:


Attachments
[PATCH] Search Fix (1.69 KB, patch)
2009-10-23 16:03 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff
[PATCH] Behavior in Comment #3 (1.84 KB, patch)
2009-10-23 16:50 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2009-10-23 15:58:31 PDT
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
Comment 1 Joseph Pecoraro 2009-10-23 16:03:44 PDT
Created attachment 41759 [details]
[PATCH] Search Fix
Comment 2 Joseph Pecoraro 2009-10-23 16:22:51 PDT
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.
Comment 3 Joseph Pecoraro 2009-10-23 16:49:02 PDT
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
Comment 4 Joseph Pecoraro 2009-10-23 16:50:51 PDT
Created attachment 41764 [details]
[PATCH] Behavior in Comment #3
Comment 5 Joseph Pecoraro 2009-10-23 17:00:23 PDT
Ugh. The "this.currentQuery !== query", is not necessary and I will remove it before I land if this gets r+.
Comment 6 Joseph Pecoraro 2009-10-23 17:27:32 PDT
The currentQuery comparison was needed for certain cases and was okay to leave in.

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