WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
18548
Web Inspector freezes if a search fires with a small number of letters
https://bugs.webkit.org/show_bug.cgi?id=18548
Summary
Web Inspector freezes if a search fires with a small number of letters
Anthony Ricaud
Reported
2008-04-17 11:07:34 PDT
If you type less than 3 letters, the search is performed and is really slow on a normal document. The search should wait for more letters or be activated by hitting the Enter button.
Attachments
Proposed patch
(3.15 KB, patch)
2008-05-26 17:42 PDT
,
Anthony Ricaud
no flags
Details
Formatted Diff
Diff
Proposed patch
(3.14 KB, patch)
2008-05-26 18:02 PDT
,
Anthony Ricaud
timothy
: review-
Details
Formatted Diff
Diff
Patch with comments addressed
(3.11 KB, patch)
2008-08-05 16:17 PDT
,
Anthony Ricaud
timothy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Anthony Ricaud
Comment 1
2008-05-20 16:51:59 PDT
Steps to reproduce 1. Go to
http://digg.com
2. Open the inspector 3. Type "a" in the search field The inspector is now freezing while performing the search. 4. In the search results, select "<head>" The inspector is also freezing. I think the first freeze is normal (lots of things to retrieve) but the user should opt-in for this search. However, the freeze when selecting a node is not normal.
Anthony Ricaud
Comment 2
2008-05-26 17:42:30 PDT
Created
attachment 21355
[details]
Proposed patch This should solve the first problem (freeze on search). I'm not sure this patch follows code conventions.
Anthony Ricaud
Comment 3
2008-05-26 18:02:10 PDT
Created
attachment 21356
[details]
Proposed patch e becomes event and no braces for single lines if statements. Thanks pewtermoose.
mitz
Comment 4
2008-05-26 20:08:17 PDT
I think that in the long-term, it would be best to perform long searches without locking up the UI.
Adam Roben (:aroben)
Comment 5
2008-05-27 12:02:49 PDT
Comment on
attachment 21356
[details]
Proposed patch I don't think it's a good idea to have searching sometimes happen automatically and sometimes happen only when pressing Enter.
Anthony Ricaud
Comment 6
2008-05-27 12:51:28 PDT
In the ideal situation, the search will be performed without locking the UI. In this case, this patch is unnecessary. However, the actual behaviour is not providing enough feedback. The options are providing search only when pressing Enter (for a consistent behaviour) or the behaviour in the patch (easier to search for long strings without pressing Enter). For the second option, a feedback a la Spotlight could help informing the user about what's happening.
Timothy Hatcher
Comment 7
2008-05-28 09:18:21 PDT
I think we should change it to search only when pressing Enter. That should give a better experience while allowing all lengths of text. This would only require removing incremental="incremental" and using the "search" event.
Matt Lilek
Comment 8
2008-05-28 15:02:42 PDT
(In reply to
comment #7
)
> I think we should change it to search only when pressing Enter. That should > give a better experience while allowing all lengths of text. > > This would only require removing incremental="incremental" and using the > "search" event. >
Or perhaps only start the search after typing has paused for a certain amount of time?
Anthony Ricaud
Comment 9
2008-05-28 15:34:05 PDT
We should not forget about step 4. I see 3s for addRange() and focus() in "set currentFocusElement" There's 802 calls to WebInspector.ResourcesPanel.refreshResource done in 1s.
Bjorn Tipling
Comment 10
2008-07-02 13:38:23 PDT
This bug is a deal breaker for me. Hope it gets quashed. Good luck!
Eric Seidel (no email)
Comment 11
2008-08-01 15:01:10 PDT
Comment on
attachment 21356
[details]
Proposed patch Assigning to Tim to review. Adam also knows this code well.
Timothy Hatcher
Comment 12
2008-08-01 15:12:58 PDT
Comment on
attachment 21356
[details]
Proposed patch + var forceSearch = event.keyCode == 13; You should use event.keyIdentifier === "Enter" here. + if (!forceSearch && this.lastQuery && this.lastQuery == query) You should use === here. Otherwise I will r+ it.
Anthony Ricaud
Comment 13
2008-08-05 16:17:50 PDT
Created
attachment 22664
[details]
Patch with comments addressed
Timothy Hatcher
Comment 14
2008-08-07 20:56:44 PDT
Landed in
r35632
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug