Bug 34251 - Web Inspector: [REGRESSION] Search doesn't continue highlighting elements on hitting 'Enter' key
: Web Inspector: [REGRESSION] Search doesn't continue highlighting elements on ...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Pavel Feldman
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-28 02:47 PST by Pavel Feldman
Modified: 2010-02-15 02:34 PST (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed change. (14.05 KB, patch)
2010-02-14 09:34 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
[PATCH] Now setting highlight in updateTitle. (15.43 KB, patch)
2010-02-14 14:01 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-01-28 02:47:27 PST
1. Navigate to http://www.google.com
2. Right click on the webpage and select Inspect element
3. Type "google" in the search box
4. Hit Enter a couple of times

What is the expected output? 
On second hit of enter key, search should move to the second element

What do you see instead?
It selects the first element code

http://code.google.com/p/chromium/issues/detail?id=33114
Comment 1 Pavel Feldman 2010-02-14 09:34:35 PST
Created attachment 48727 [details]
[PATCH] Proposed change.

Search on elements panel still has node granularity, so this change will highlight all occurrences of the search query within the node. I added FIXME for this and some other stuff. It is anyway better than what is now on ToT.
Comment 2 Timothy Hatcher 2010-02-14 13:04:51 PST
Comment on attachment 48727 [details]
[PATCH] Proposed change.

What happens if a matched node is updated (attribute changes, etc) during a search? Then canceling will revert the changes with originalInnerHTML.

Also node updates will blow away highlights for that node.

Maybe instead of doing innerHTML swizzling thins highlighting/unhighlighting should happen in updateTille so changes to the DOM will work.
Comment 3 Pavel Feldman 2010-02-14 13:28:07 PST
(In reply to comment #2)
> (From update of attachment 48727 [details])
> What happens if a matched node is updated (attribute changes, etc) during a
> search? Then canceling will revert the changes with originalInnerHTML.
> Also node updates will blow away highlights for that node.
> 
> Maybe instead of doing innerHTML swizzling thins highlighting/unhighlighting
> should happen in updateTille so changes to the DOM will work.

Heh. You are right - for a moment I thought that treeItem is going to be entirely re-created, but now I see that only title is being rebuilt.
Comment 4 Pavel Feldman 2010-02-14 14:01:11 PST
Created attachment 48733 [details]
[PATCH] Now setting highlight in updateTitle.

Changes to attributes are now handled properly. Changes to text nodes are processed as remove/add though. As a result, newly created nodes are absent in the searchResults array even if they contain matching text. But that is a general issue that has been there for all the times. The right solution would be to restart search from the current occurrence in case of changes to dom.
Comment 5 Timothy Hatcher 2010-02-14 14:22:06 PST
Comment on attachment 48733 [details]
[PATCH] Now setting highlight in updateTitle.

I think it should be _highlightSearchResults and highlightSearchResults? Since they highlight multiple matches.
Comment 6 Pavel Feldman 2010-02-15 02:34:57 PST
(In reply to comment #5)
> (From update of attachment 48733 [details])
> I think it should be _highlightSearchResults and highlightSearchResults? Since
> they highlight multiple matches.

Done.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/ElementsTreeOutline.js
	M	WebCore/inspector/front-end/InjectedScript.js
	M	WebCore/inspector/front-end/SourceFrame.js
	M	WebCore/inspector/front-end/TextViewer.js
	M	WebCore/inspector/front-end/textViewer.css
	M	WebCore/inspector/front-end/utilities.js
Committed r54770