Chromium Code Reviews
Help | Chromium Project | Sign in
(42)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by aknudsen
Modified:
2 years, 2 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 66 (11 generated)
aknudsen
2 years, 5 months 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 ...
2 years, 5 months 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, ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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.
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 5 months 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 ...
2 years, 4 months 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 ! ...
2 years, 4 months 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: > ...
2 years, 4 months 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 ...
2 years, 4 months 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 ...
2 years, 4 months 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 ...
2 years, 4 months 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: ...
2 years, 4 months ago (2014-11-19 06:55:47 UTC) #34
lushnikov
> When considering merging tests, I thought about how they differ in setup logic. > ...
2 years, 4 months 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 ...
2 years, 4 months 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 ...
2 years, 3 months 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 ...
2 years, 3 months 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: ...
2 years, 3 months 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 ...
2 years, 3 months 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 ...
2 years, 3 months 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 ...
2 years, 3 months 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'? ...
2 years, 3 months 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 ...
2 years, 3 months 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: ...
2 years, 3 months 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: ...
2 years, 3 months 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 ...
2 years, 3 months 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 ...
2 years, 3 months 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: // ...
2 years, 3 months 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 ...
2 years, 3 months 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, ...
2 years, 3 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 ...
2 years, 3 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
2 years, 3 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 ...
2 years, 3 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, ...
2 years, 3 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
2 years, 3 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)
2 years, 3 months ago (2014-12-25 22:20:03 UTC) #61
pfeldman
lgtm
2 years, 2 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
2 years, 2 months ago (2015-01-12 14:45:15 UTC) #65
commit-bot: I haz the power
2 years, 2 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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46