Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(288)

Issue 676193002: Navigate between individual search matches in DevTools console (Closed)

Created:
6 years, 1 month ago by aknudsen
Modified:
5 years, 11 months ago
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Navigate between individual search matches in DevTools console Rather than the original DevTools console search behaviour, which is to navigate between lines containing search matches, highlight all matches and navigate between individual matches irrespective of containing lines. Please note that the .current-search-result CSS class should be revised by a designer. BUG=426366 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188234

Patch Set 1 #

Total comments: 22

Patch Set 2 : Revision #1 #

Patch Set 3 : Add test; fix scrolling to current match #

Patch Set 4 : Mark current search results by modifying CSS classes #

Patch Set 5 : Scroll tree elements into view #

Patch Set 6 : Add tests #

Total comments: 102

Patch Set 7 : Highlight all matches per message at once #

Patch Set 8 : Rebase on origin/master #

Patch Set 9 : Consolidate test suites #

Total comments: 8

Patch Set 10 : Improve test #

Total comments: 7

Patch Set 11 : Rebase on origin/master #

Patch Set 12 : Call clearHighlights once per message #

Total comments: 2

Patch Set 13 : Remove comments #

Patch Set 14 : Synchronize with origin #

Unified diffs Side-by-side diffs Delta from patch set Stats (+545 lines, -72 lines) Patch
A LayoutTests/inspector/console/console-search.html View 1 2 3 4 5 6 7 8 9 1 chunk +155 lines, -0 lines 0 comments Download
A LayoutTests/inspector/console/console-search-expected.txt View 1 2 3 4 5 6 7 8 9 1 chunk +236 lines, -0 lines 0 comments Download
M Source/devtools/front_end/console/ConsoleView.js View 1 2 3 4 5 6 7 8 9 10 11 12 12 chunks +116 lines, -56 lines 0 comments Download
M Source/devtools/front_end/console/ConsoleViewMessage.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +24 lines, -12 lines 0 comments Download
M Source/devtools/front_end/console/consoleView.css View 1 2 3 4 5 6 7 8 1 chunk +7 lines, -0 lines 0 comments Download
M Source/devtools/front_end/elements/ElementsPanel.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M Source/devtools/front_end/ui/UIUtils.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 66 (11 generated)
aknudsen
6 years, 1 month ago (2014-10-25 22:29:47 UTC) #2
robwu
I've reviewed your code and glanced over the logic, but I haven't looked at the ...
6 years, 1 month ago (2014-10-25 23:13:00 UTC) #3
aknudsen
On 2014/10/25 23:13:00, robwu wrote: > I've reviewed your code and glanced over the logic, ...
6 years, 1 month ago (2014-10-26 10:58:38 UTC) #4
aknudsen
https://codereview.chromium.org/676193002/diff/1/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/1/Source/devtools/front_end/console/ConsoleView.js#newcode579 Source/devtools/front_end/console/ConsoleView.js:579: if (this._searchRegex && this._searchMessage(this._visibleViewMessages.length-1)) { On 2014/10/25 23:12:59, robwu ...
6 years, 1 month ago (2014-10-26 11:22:41 UTC) #7
aknudsen
On 2014/10/25 23:13:00, robwu wrote: > And these features surely need some test coverage. Would ...
6 years, 1 month ago (2014-10-26 18:43:21 UTC) #8
robwu
On 2014/10/26 18:43:21, aknudsen wrote: > On 2014/10/25 23:13:00, robwu wrote: > > And these ...
6 years, 1 month ago (2014-10-26 19:03:02 UTC) #9
aknudsen
On 2014/10/26 19:03:02, robwu wrote: > 1. Get Chromium, see > http://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html Can I execute ...
6 years, 1 month ago (2014-10-26 19:36:34 UTC) #10
robwu
On 2014/10/26 19:36:34, aknudsen wrote: > On 2014/10/26 19:03:02, robwu wrote: > > 1. Get ...
6 years, 1 month ago (2014-10-26 20:05:56 UTC) #11
aknudsen
On 2014/10/26 20:05:56, robwu wrote: > On 2014/10/26 19:36:34, aknudsen wrote: > > On 2014/10/26 ...
6 years, 1 month ago (2014-10-26 21:24:10 UTC) #12
robwu
On 2014/10/26 21:24:10, aknudsen wrote: > On 2014/10/26 20:05:56, robwu wrote: > > On 2014/10/26 ...
6 years, 1 month ago (2014-10-26 21:30:19 UTC) #13
lushnikov
I've applied the patch and played with it. It's a great start! However, there are ...
6 years, 1 month ago (2014-10-27 08:16:37 UTC) #15
lushnikov
Regarding the console-search-reveals-messages.html test, here's an explanation how things work https://gist.github.com/aslushnikov/f1bf449ebc955435c38a Hope this helps.
6 years, 1 month ago (2014-10-27 09:14:40 UTC) #16
aknudsen
On 2014/10/26 19:03:02, robwu wrote: > On 2014/10/26 18:43:21, aknudsen wrote: > > Would you ...
6 years, 1 month ago (2014-10-27 17:37:43 UTC) #17
aknudsen
On 2014/10/27 08:16:37, lushnikov wrote: > I've applied the patch and played with it. It's ...
6 years, 1 month ago (2014-10-27 22:20:22 UTC) #18
lushnikov
On 2014/10/27 17:37:43, aknudsen wrote: > On 2014/10/26 19:03:02, robwu wrote: > > On 2014/10/26 ...
6 years, 1 month ago (2014-10-28 04:43:55 UTC) #19
aknudsen
On 2014/10/28 04:43:55, lushnikov wrote: > On 2014/10/27 17:37:43, aknudsen wrote: > > On 2014/10/26 ...
6 years, 1 month ago (2014-10-28 11:18:04 UTC) #20
aknudsen
On 2014/10/27 09:14:40, lushnikov wrote: > Regarding the console-search-reveals-messages.html test, here's an explanation > how ...
6 years, 1 month ago (2014-10-29 22:08:22 UTC) #21
aknudsen
On 2014/10/27 08:16:37, lushnikov wrote: > I've applied the patch and played with it. It's ...
6 years, 1 month ago (2014-10-29 22:09:41 UTC) #22
lushnikov
On 2014/10/29 22:09:41, aknudsen wrote: > On 2014/10/27 08:16:37, lushnikov wrote: > > I've applied ...
6 years, 1 month ago (2014-10-30 06:09:35 UTC) #23
aknudsen
On 2014/10/30 06:09:35, lushnikov wrote: > On 2014/10/29 22:09:41, aknudsen wrote: > > On 2014/10/27 ...
6 years, 1 month ago (2014-10-30 09:03:54 UTC) #24
lushnikov
On 2014/10/30 09:03:54, aknudsen wrote: > On 2014/10/30 06:09:35, lushnikov wrote: > > On 2014/10/29 ...
6 years, 1 month ago (2014-10-30 09:20:53 UTC) #25
aknudsen
On 2014/10/30 09:20:53, lushnikov wrote: > On 2014/10/30 09:03:54, aknudsen wrote: > > On 2014/10/30 ...
6 years, 1 month ago (2014-10-30 09:29:48 UTC) #26
lushnikov
I've applied your patch one more time. There's still a lag when i search for ...
6 years, 1 month ago (2014-11-07 16:27:19 UTC) #27
apavlov
https://codereview.chromium.org/676193002/diff/140001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/140001/Source/devtools/front_end/console/ConsoleView.js#newcode1223 Source/devtools/front_end/console/ConsoleView.js:1223: * @return {!string} Non-object types should not have ! ...
6 years, 1 month ago (2014-11-07 16:30:07 UTC) #29
aknudsen
https://codereview.chromium.org/676193002/diff/140001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/140001/Source/devtools/front_end/console/ConsoleView.js#newcode51 Source/devtools/front_end/console/ConsoleView.js:51: this._regexMatchRanges = []; On 2014/11/07 16:27:16, lushnikov wrote: > ...
6 years, 1 month ago (2014-11-08 11:49:51 UTC) #30
aknudsen
https://codereview.chromium.org/676193002/diff/140001/LayoutTests/inspector/console/console-search-can-highlight-current-match.html File LayoutTests/inspector/console/console-search-can-highlight-current-match.html (right): https://codereview.chromium.org/676193002/diff/140001/LayoutTests/inspector/console/console-search-can-highlight-current-match.html#newcode22 LayoutTests/inspector/console/console-search-can-highlight-current-match.html:22: setTimeout(InspectorTest.completeTest.bind(InspectorTest), 2000); On 2014/11/07 16:27:16, lushnikov wrote: > We ...
6 years, 1 month ago (2014-11-08 12:00:53 UTC) #31
aknudsen
https://codereview.chromium.org/676193002/diff/140001/LayoutTests/inspector/console/console-search-can-highlight-current-match.html File LayoutTests/inspector/console/console-search-can-highlight-current-match.html (right): https://codereview.chromium.org/676193002/diff/140001/LayoutTests/inspector/console/console-search-can-highlight-current-match.html#newcode22 LayoutTests/inspector/console/console-search-can-highlight-current-match.html:22: setTimeout(InspectorTest.completeTest.bind(InspectorTest), 2000); On 2014/11/07 16:27:16, lushnikov wrote: > We ...
6 years, 1 month ago (2014-11-09 00:02:14 UTC) #32
aknudsen
https://codereview.chromium.org/676193002/diff/140001/LayoutTests/inspector/console/console-search-can-jump-backward-between-matches.html File LayoutTests/inspector/console/console-search-can-jump-backward-between-matches.html (right): https://codereview.chromium.org/676193002/diff/140001/LayoutTests/inspector/console/console-search-can-jump-backward-between-matches.html#newcode51 LayoutTests/inspector/console/console-search-can-jump-backward-between-matches.html:51: function testFindMatches(next) On 2014/11/07 16:27:16, lushnikov wrote: > Could ...
6 years, 1 month ago (2014-11-09 00:09:46 UTC) #33
lushnikov
https://codereview.chromium.org/676193002/diff/140001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/140001/Source/devtools/front_end/console/ConsoleView.js#newcode959 Source/devtools/front_end/console/ConsoleView.js:959: var message = this._visibleViewMessages[ On 2014/11/08 11:49:50, aknudsen wrote: ...
6 years, 1 month ago (2014-11-19 06:55:47 UTC) #34
lushnikov
> When considering merging tests, I thought about how they differ in setup logic. > ...
6 years, 1 month ago (2014-11-19 06:57:00 UTC) #35
aknudsen
On 2014/11/19 06:57:00, lushnikov wrote: > > When considering merging tests, I thought about how ...
6 years ago (2014-11-24 08:07:44 UTC) #36
aknudsen
On 2014/11/24 08:07:44, aknudsen wrote: > On 2014/11/19 06:57:00, lushnikov wrote: > > > When ...
6 years ago (2014-12-02 22:56:42 UTC) #37
lushnikov
Thank you for the effort! Just a few last nits before we can have this ...
6 years ago (2014-12-04 16:07:34 UTC) #38
aknudsen
https://codereview.chromium.org/676193002/diff/200001/LayoutTests/inspector/console/console-search.html File LayoutTests/inspector/console/console-search.html (right): https://codereview.chromium.org/676193002/diff/200001/LayoutTests/inspector/console/console-search.html#newcode62 LayoutTests/inspector/console/console-search.html:62: // Find first match On 2014/12/04 16:07:34, lushnikov wrote: ...
6 years ago (2014-12-05 22:30:02 UTC) #39
aknudsen
I haven't tested my latest changes, since the current Chromium head isn't buildable :( I ...
6 years ago (2014-12-05 22:36:49 UTC) #40
robwu
On 2014/12/05 22:36:49, aknudsen wrote: > I haven't tested my latest changes, since the current ...
6 years ago (2014-12-05 23:05:11 UTC) #41
aknudsen
On Sat, Dec 6, 2014 at 12:05 AM, <rob@robwu.nl> wrote: > On 2014/12/05 22:36:49, aknudsen ...
6 years ago (2014-12-05 23:52:21 UTC) #42
aknudsen
Nah, still fails: ./../third_party/WebKit/Source/core/inspector/ScriptProfile.cpp:70:32: error: no member named 'GetHitLineCount' in 'v8::CpuProfileNode'; did you mean 'GetHitCount'? ...
6 years ago (2014-12-06 00:21:46 UTC) #43
lushnikov
First of all, sorry for the long delay, and thank you for all the work ...
6 years ago (2014-12-17 13:21:23 UTC) #44
aknudsen
https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js#newcode1023 Source/devtools/front_end/console/ConsoleView.js:1023: var message = this._visibleViewMessages[matchRange.messageIndex]; On 2014/12/17 13:21:23, lushnikov wrote: ...
6 years ago (2014-12-21 17:58:24 UTC) #45
aknudsen
https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js#newcode1023 Source/devtools/front_end/console/ConsoleView.js:1023: var message = this._visibleViewMessages[matchRange.messageIndex]; On 2014/12/17 13:21:23, lushnikov wrote: ...
6 years ago (2014-12-21 20:00:09 UTC) #46
aknudsen
Btw guys, I'm not able to test in Canary at this point. I get an ...
6 years ago (2014-12-21 20:04:40 UTC) #47
lushnikov
On 2014/12/21 20:04:40, aknudsen wrote: > Btw guys, I'm not able to test in Canary ...
6 years ago (2014-12-23 13:41:10 UTC) #48
lushnikov
lgtm given the last nit fixed yay! :) https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js#newcode1051 Source/devtools/front_end/console/ConsoleView.js:1051: // ...
6 years ago (2014-12-23 13:43:33 UTC) #49
robwu
One more nit from me. https://codereview.chromium.org/676193002/diff/260001/Source/devtools/front_end/inspectorCommon.css File Source/devtools/front_end/inspectorCommon.css (right): https://codereview.chromium.org/676193002/diff/260001/Source/devtools/front_end/inspectorCommon.css#newcode154 Source/devtools/front_end/inspectorCommon.css:154: Undo this unnecessary space ...
6 years ago (2014-12-23 13:53:35 UTC) #50
aknudsen
On 2014/12/23 13:41:10, lushnikov wrote: > On 2014/12/21 20:04:40, aknudsen wrote: > > Btw guys, ...
5 years, 12 months ago (2014-12-24 10:52:20 UTC) #51
aknudsen
All done I think. Merry Christmas! https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/676193002/diff/220001/Source/devtools/front_end/console/ConsoleView.js#newcode1051 Source/devtools/front_end/console/ConsoleView.js:1051: // Scroll the ...
5 years, 12 months ago (2014-12-24 11:08:07 UTC) #52
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/676193002/280001
5 years, 12 months ago (2014-12-24 22:42:42 UTC) #54
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/23038) win_blink_compile_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/builds/30681) win_gpu ...
5 years, 12 months ago (2014-12-24 22:47:40 UTC) #56
robwu
apavlov: Could you put your LGTM on this CL? Lushnikov has already LGed this CL, ...
5 years, 12 months ago (2014-12-24 22:51:05 UTC) #57
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/676193002/280001
5 years, 12 months ago (2014-12-25 22:14:12 UTC) #59
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/23076)
5 years, 12 months ago (2014-12-25 22:20:03 UTC) #61
pfeldman
lgtm
5 years, 11 months ago (2015-01-12 13:24:29 UTC) #63
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/676193002/300001
5 years, 11 months ago (2015-01-12 14:45:15 UTC) #65
commit-bot: I haz the power
5 years, 11 months ago (2015-01-12 17:28:29 UTC) #66
Message was sent while issue was closed.
Committed patchset #14 (id:300001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188234

Powered by Google App Engine
This is Rietveld 408576698