Summary: | Web Inspector: highlight occurences of word in DefaultTextEditor | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andrey Lushnikov <lushnikov> | ||||||||||||||||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Andrey Lushnikov <lushnikov> | ||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||
Severity: | Normal | CC: | apavlov, buildbot, dglazkov, keishi, loislo, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||||||||
Attachments: |
|
Description
Andrey Lushnikov
2012-12-20 03:14:57 PST
Created attachment 180314 [details]
Patch
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... Clarifying title: highlighting not only identifier occurrences, but any word occurrences. Created attachment 182761 [details]
Patch
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. Created attachment 182771 [details]
Patch
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 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 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 Created attachment 182948 [details]
Patch
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 Created attachment 182958 [details]
word-highlight-screenshot
What's happening when you select some word
Created attachment 182979 [details]
Patch
Created attachment 182981 [details]
feature demonstration
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; 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. Created attachment 184235 [details]
Patch
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 ? 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). 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 Created attachment 184431 [details]
Patch
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. Created attachment 184487 [details]
Patch
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 Created attachment 184503 [details]
Patch
Comment on attachment 184503 [details]
Patch
re-submitted last patch
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 Created attachment 184696 [details]
Patch
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 Created attachment 184741 [details]
Patch
Comment on attachment 184741 [details] Patch Clearing flags on attachment: 184741 Committed r140828: <http://trac.webkit.org/changeset/140828> All reviewed patches have been landed. Closing bug. |