Bug 34251 – Web Inspector: [REGRESSION] Search doesn't continue highlighting elements on hitting 'Enter' key

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

[Pavel Feldman]

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

[Pavel Feldman]

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.

[Timothy Hatcher]

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.

[Pavel Feldman]

(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.

[Pavel Feldman]

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.

[Timothy Hatcher]

Comment on attachment 48733 [details]
[PATCH] Now setting highlight in updateTitle.

I think it should be _highlightSearchResults and highlightSearchResults? Since they highlight multiple matches.

[Pavel Feldman]

(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