1522858 - "Find" command in source pane skips every other match

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Lars T Hansen [:lth]]

STR:
Untar the attached file, it contains test.html and test.js.
Open test.html in a new tab in the browser (from the file system is fine), select the tab for inspection in about:debugging.
Expand file tree in inspector until you can select test.js.
With focus in the source code pane, search (Ctrl-F / Cmd-F) for the string 'f(', for which there are five matches.
Press Ctrl-G / Cmd-G to find the next.

Expected result: skip to next match in source

Actual result: skips over the next match and finds the one after that.

This is completely repeatable, I found it in a much larger program I'm debugging elsewhere.

Comment 1

[Soledad Penades [:sole] [:spenades]]

For some reason, I can't see any attached file, but this sounds to me like a debugger, not about:debugging issue.

Did you really attach the file?

Comment 2

[Jason Laster [:jlast]]

I see this if i go to http://todomvc.com/examples/backbone/ and search this in todo-view.js.

Comment 3

[Lars T Hansen [:lth]]

Hm, sorry about the missing file.

You can repro by opening a debugger on this bugzilla page and selecting eg the "dropdown.js" file and then searching for "function"; it'll be obvious what's going on.

Comment 4

[Lars T Hansen [:lth]]

Note that if there are only two matches for what you're looking for, the bug will make it look like Ctrl-G isn't working at all.

The behavior is the same for disassembled wasm in text panes, by the way, so it looks like a general text search problem.

Comment 5

[David Walsh :davidwalsh]

Attachment: Test run of "this"

Unless I read the STR incorrectly, I cannot reproduce this issue in Nightly.

Comment 6

[Lars T Hansen [:lth]]

This reproduces for me in this morning's nightly on both Linux and Mac.

STR:

• Update to latest Nightly
• Open a new tab to bugzilla.mozilla.org
• From the hamburger menu, choose Web Developer | Debugger
• In the sources pane, expand the static/v2019... folder (presumably this name changes from time to time)
• Under that, expand the JS pane
• select "dropdown.js", the source should show in the source pane of the debugger
• in the source pane, press Ctrl-F or Cmd-F to start searching
• search for "this" or "function" or something else common
• press Ctrl-G / Cmd-G to find the next search. DO NOT click on the "next match" / "previous match" arrows
• it will be obvious that the search skips every other occurrence, because the source pane highlights all of the matches but go-to-next with the keyboard skips every other one

Note again, this is with the keyboard. Clicking on the "next" / "previous" match icons does not skip every other one, it finds all the matches.

Comment 7

[Chujun Lu]

Interested in working for this. I believe this bug is related to a new issue on Github: https://github.com/firefox-devtools/debugger/issues/8180. Since Ctrl-G / Cmd-G skips every other match, when there are only two matches, Ctrl-G / Cmd-G would jump to the same one, which looks like the shortcut isn't working.

Comment 8

[Jason Laster [:jlast]]

Sure Chujun

Comment 9

[Chujun Lu]

Attachment: Bug 1522858 Remove duplication of finding next/previous matching result for keyboard shortcuts r=jlast

Cause of the bug:

In src/components/Editor/index.js, “sourceSearch.search.again.key2” (CmdOrCtrl+G) calls onSearchAgain, which then calls this.props.traverseResults.
In src/components/Editor/SearchBar.js, the same key calls this.props.traverseResults as well.

Thus, the same keyboard shortcut calls the same function twice. So does “sourceSearch.search.againPrev.key2” (CmdOrCtrl+Shift+G).

In pull request #1656 (https://github.com/firefox-devtools/debugger/pull/1656), a feature of double-clicking a text token to highlight it was added. Another feature of using CmdorCtrl+G to jump to the next matching result was also added.

The double-clicking feature is no longer here as I observe in the debugger, but the traversing to other matching results function has been kept.

Keyboard shortcuts related to the file search box are all in SearchBar.js. So I decided to keep the one in SearchBar.js, and remove the one in index.js.

Comment 10

[jaril]

Attachment: Bug 1522858 - Fixed Watch Expression vertical alignment r=davidwalsh

Set the expression container to 20px

Comment 11

[Phabricator Automation]

Comment on attachment 9058119 [details]
Bug 1522858 - Fixed Watch Expression vertical alignment r=davidwalsh

Revision D27442 was moved to bug 1544240. Setting attachment 9058119 [details] to obsolete.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a890f8d88d5
Remove duplication of finding next/previous matching result for keyboard shortcuts

Comment 13

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/5a890f8d88d5