Bug 105523 – Web Inspector: highlight occurences of word in DefaultTextEditor

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

[Andrey Lushnikov]

It would be convenient to highlight all occurrences of some identifier in a javascript code when user selects one of them (via text selection). 
This is done per-word in a Sublime Text 2 (i.e. it highlights word occurrences in text)

[Andrey Lushnikov]

Created attachment 180314 [details]
Patch

[Pavel Feldman]

Comment on attachment 180314 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1344
> +    function checkSelectedIdentifier(event) {

This should be a prototype method that you bind and pass into listeners.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1346
> +        if (selection.isCollapsed)

We already have selection stored in a variable in mainPanel

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1353
> +        // The following conditions will:

WebKit strongly advises not using comments. We only use them when absolutely necessary.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1369
> +            if (className === "webkit-javascript-ident") {

Do not use "javascript" word in editor. What about css variables?

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1375
> +    this.element.addEventListener("mousemove", checkSelectedIdentifier.bind(this), false);

"click"

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1377
> +    this.element.addEventListener("keyup", checkSelectedIdentifier.bind(this), false);

we already track text mutations, why bothering about the keyboard events?

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1391
> +        var elements = document.querySelectorAll(".webkit-javascript-ident");

Just repaintAll

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1401
> +            var elements = document.querySelectorAll(".webkit-selected-identifier");

ditto

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2151
> +        if (span.className === "webkit-javascript-ident" && this._selectedIdentifier && this._selectedIdentifier === content)

javascript word again...

[Andrey Lushnikov]

Clarifying title: highlighting not only identifier occurrences, but any word occurrences.

[Andrey Lushnikov]

Created attachment 182761 [details]
Patch

[Pavel Feldman]

Comment on attachment 182761 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3046
> +            return this._deselect();

Although I would love to act alike, we don't do that.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3068
> +    _select: function(selectedWord)

this naming is misleading - it does not mutate selection.

[Andrey Lushnikov]

Created attachment 182771 [details]
Patch

[WebKit Review Bot]

Comment on attachment 182771 [details]
Patch

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

New failing tests:
inspector/editor/text-editor-highlight-token.html

[Build Bot]

Comment on attachment 182771 [details]
Patch

Attachment 182771 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15845990

New failing tests:
inspector/editor/text-editor-highlight-token.html

[Build Bot]

Comment on attachment 182771 [details]
Patch

Attachment 182771 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15900409

New failing tests:
inspector/editor/text-editor-highlight-token.html

[Andrey Lushnikov]

Created attachment 182948 [details]
Patch

[Build Bot]

Comment on attachment 182948 [details]
Patch

Attachment 182948 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15901413

New failing tests:
inspector/editor/text-editor-highlight-token.html

[Andrey Lushnikov]

Created attachment 182958 [details]
word-highlight-screenshot

What's happening when you select some word

[Andrey Lushnikov]

Created attachment 182979 [details]
Patch

[Andrey Lushnikov]

Created attachment 182981 [details]
feature demonstration

[Pavel Feldman]

Comment on attachment 182979 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3043
> +        if (!range) {

if (!this._muteSelectionListener)

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3055
> +        if (selectedText == this._selectedWord)

===

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3077
> +        this._mainPanel.highlightRegex(this._activeRegex, "text-editor-token-highlight")

this._muteSelectionListener = true;
this._mainPanel.highlightRegex(this._activeRegex, "text-editor-token-highlight");
delete this._muteSelectionListener;

[Andrey Lushnikov]

Comment on attachment 182979 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3043
>> +        if (!range) {
> 
> if (!this._muteSelectionListener)

_muteSelectionListener doesn't work here (see below).

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3055
>> +        if (selectedText == this._selectedWord)
> 
> ===

my bad; fixed.

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3077
>> +        this._mainPanel.highlightRegex(this._activeRegex, "text-editor-token-highlight")
> 
> this._muteSelectionListener = true;
> this._mainPanel.highlightRegex(this._activeRegex, "text-editor-token-highlight");
> delete this._muteSelectionListener;

this doesn't work because events happen on the next scheduled frame.

[Andrey Lushnikov]

Created attachment 184235 [details]
Patch

[Pavel Feldman]

Comment on attachment 184235 [details]
Patch

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

> LayoutTests/inspector/editor/text-editor-highlight-token.html:35
> +        setTimeout(step2);

why setTimeout ?

[Andrey Lushnikov]

Comment on attachment 184235 [details]
Patch

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

>> LayoutTests/inspector/editor/text-editor-highlight-token.html:35
>> +        setTimeout(step2);
> 
> why setTimeout ?

That's because setSelection triggers "document.selectionChange" listeners which gonna be executed on next frame (and token highlighter is one of those listeners).

[Build Bot]

Comment on attachment 184235 [details]
Patch

Attachment 184235 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16063935

New failing tests:
svg/as-image/img-preserveAspectRatio-support-2.html

[Andrey Lushnikov]

Created attachment 184431 [details]
Patch

[Pavel Feldman]

Comment on attachment 184431 [details]
Patch

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

> LayoutTests/inspector/editor/text-editor-highlight-token.html:45
> +    setTimeout(step1);

Bump it above step1. You should use selection listener here.

[Andrey Lushnikov]

Created attachment 184487 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184487 [details]
Patch

Rejecting attachment 184487 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 184487, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
atch: **** Can't create file /tmp/ppQlOiba : No space left on device
fatal: pathspec 'LayoutTests/inspector/editor/text-editor-highlight-token-expected.txt' did not match any files
Failed to git add LayoutTests/inspector/editor/text-editor-highlight-token-expected.txt. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 448.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Pavel Feldman']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16097229

[Andrey Lushnikov]

Created attachment 184503 [details]
Patch

[Andrey Lushnikov]

Comment on attachment 184503 [details]
Patch

re-submitted last patch

[WebKit Review Bot]

Comment on attachment 184503 [details]
Patch

Rejecting attachment 184503 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 184503, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
atch: **** Can't create file /tmp/pp2s7yc3 : No space left on device
fatal: pathspec 'LayoutTests/inspector/editor/text-editor-highlight-token-expected.txt' did not match any files
Failed to git add LayoutTests/inspector/editor/text-editor-highlight-token-expected.txt. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 448.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Pavel Feldman']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16080734

[Andrey Lushnikov]

Created attachment 184696 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184696 [details]
Patch

Rejecting attachment 184696 [details] from commit-queue.

New failing tests:
fast/frames/parser-append-subframe-count.html
Full output: http://queues.webkit.org/results/16121280

[Andrey Lushnikov]

Created attachment 184741 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184741 [details]
Patch

Clearing flags on attachment: 184741

Committed r140828: <http://trac.webkit.org/changeset/140828>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.