Bug 91394 – Web Inspector: implement search / replace in source files (behind experiment flag)

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

[Pavel Feldman]

See image attached.

[Pavel Feldman]

Created attachment 152537 [details]
[IMAGE] with patch applied

[Pavel Feldman]

Created attachment 152541 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 152541 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152541&action=review

> Source/WebCore/inspector/front-end/ConsolePanel.js:106
> +    jumpToNextSearchResult: function(loop)

Please annotate return value.

> Source/WebCore/inspector/front-end/ConsolePanel.js:119
> +    jumpToPreviousSearchResult: function(loop)

Ditto.

> Source/WebCore/inspector/front-end/ElementsPanel.js:499
> +    jumpToNextSearchResult: function(loop)

Ditto.

> Source/WebCore/inspector/front-end/ElementsPanel.js:518
> +    jumpToPreviousSearchResult: function(loop)

Ditto.

> Source/WebCore/inspector/front-end/SearchController.js:200
> +        this._replaceCheckboxElement.disabled = !panel.canSearchAndReplace() && !enabled;

This behavior seems odd to me.
Replace check box is currently disabled in two cases: there is no search matches and/or panel does not support replace.
In one case user can make it enabled by typing a good search query, in the other it will always stay disabled.
I think we can make it always enabled on scripts panel and keep disabled on other panels (for discovery) OR make it hidden on other panels.
I would actually like to see both of these changes.

The current behavior is also incorrect:
1. type good search query
2. enable replace
3. erase search query

Replace checkbox becomes disabled but replace input field and buttons are still shown.

> Source/WebCore/inspector/front-end/SearchController.js:331
> +        this._performSearch(query, true, false, false);

It seems to me that all this loop=false machinery does not work here when replacement does not contain search query.
When you press replace while standing on the last search match you run perform search again and it starts from the file start again.

I would actually prefer if this feature allowed replacement loop but showed a message when I pass the same line again (not the file start)

[Pavel Feldman]

(In reply to comment #3)
> (From update of attachment 152541 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152541&action=review
> 
> > Source/WebCore/inspector/front-end/ConsolePanel.js:106
> > +    jumpToNextSearchResult: function(loop)
> 
> Please annotate return value.
> 

Done.

> This behavior seems odd to me.

Fixed.

> It seems to me that all this loop=false machinery does not work here when replacement does not contain search query.

Fixed.

[Pavel Feldman]

Created attachment 152752 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 152752 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152752&action=review

> Source/WebCore/inspector/front-end/SourceFrame.js:334
> +     * @param {boolean} loop

Please remove loop from annotations.

[WebKit Review Bot]

Comment on attachment 152752 [details]
Patch

Attachment 152752 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13282149

New failing tests:
inspector/extensions/extensions-panel.html

[WebKit Review Bot]

Created attachment 152757 [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Pavel Feldman]

Committed r122847: <http://trac.webkit.org/changeset/122847>