815277 - Source editor's 'Find' and 'Jump to line' commands should use the frontend UI

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Comment 1

[Mihai Sucan [:msucan]]

Please explain.

Comment 2

[Victor Porof [:vporof][:vp]]

Accel+F, for example (or via the context menu) brings up a prompt with a textbox input. We already have this UI exposed in the frontend via the searchbox and the # operator, or simply Ctrl+Shift+F. It seems to me that having two input modes for the same functionality is redundant and confusing. I think that the 'Find' and 'Jump to line' commands should defer control to the searchbox in the debugger's frontend UI.

Comment 3

[Mihai Sucan [:msucan]]

Good point.

You can do that with the current API: just replace the context menu item to point to a debugger command that focuses the search field. Similarly for the jump to line command. You can also replace the keyboard shortcuts.

Please let me know if there's any problem.

Comment 4

[Victor Porof [:vporof][:vp]]

(In reply to Mihai Sucan [:msucan] from comment #3)

Sounds pretty straightforward, thanks for the tips!

Comment 5

[Victor Porof [:vporof][:vp]]

Attachment: v1

Also localized a bunch of commands, which, for some ancient reason, weren't.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 685349 [details] [diff] [review]
v1

Review of attachment 685349 [details] [diff] [review]:
-----------------------------------------------------------------

I like it!

::: browser/devtools/shared/LayoutHelpers.jsm
@@ +326,5 @@
>     *
>     * @param Node aElemKey
>     *        The key element to get the modifiers from.
> +   * @param boolean aAllowCloverleaf
> +   *        Pass true to avoid the cloverleaf meta-key string.

s/true/false/

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Panos Astithas [:past] from comment #6)
> Comment on attachment 685349 [details] [diff] [review]
> v1
> 
> Review of attachment 685349 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I like it!
> 
> ::: browser/devtools/shared/LayoutHelpers.jsm
> @@ +326,5 @@
> >     *
> >     * @param Node aElemKey
> >     *        The key element to get the modifiers from.
> > +   * @param boolean aAllowCloverleaf
> > +   *        Pass true to avoid the cloverleaf meta-key string.
> 
> s/true/false/

Or maybe you want to reword this a bit, because I still find it slightly confusing. How about: Pass true to use the cloverleaf symbol instead of a descriptive string.

Comment 8

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #7)
> (In reply to Panos Astithas [:past] from comment #6)
> 
> Or maybe you want to reword this a bit, because I still find it slightly
> confusing. How about: Pass true to use the cloverleaf symbol instead of a
> descriptive string.

Ok. I too, don't not hate booleans.

I'll just try this patch on Windows and Linux first, to make sure the keyboard shortcuts still work properly.

Comment 9

[Victor Porof [:vporof][:vp]]

Tested shortcuts and they work on Windows and Linux.

Comment 10

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/6a87f91c209c

Comment 11

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/mozilla-central/rev/6a87f91c209c