1682340 - Reader Mode activates erroneously when trying to activate the Search History in the Console's multi-line editor mode

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Itiel]

STR:

1. Be on this bugzilla ticket
2. Ctrl+Shift+K to open the console
3. Enable the multi-line editor mode
4. Hit F9 to enter the Search History function there

AR:
The webpage enters reader mode, and the Search History activates, both at the same time.

ER:
Assumingly don't enter reader mode, but I'm not sure what would be the criteria for not enabling it...

Comment 1

[:Gijs (he/him)]

So, funny thing, I don't get offered reader mode on this bug at time of writing (I expect it depends on the amount of text, so this might change with this comment and/or later). Also, on macOS the shortcut for history appears to be Ctrl+R, and the reader mode shortcut is command+opt+R on macOS, so there's no conflict. But I can see how this would be an issue on Windows/Linux, where the shortcuts are presumably the same...

I think devtools already calls preventDefault() at https://searchfox.org/mozilla-central/rev/8d722de75886d6bffc116772a1db8854e34ee6a7/devtools/client/webconsole/components/Input/ReverseSearchInput.js#116-118 . This is done from the keydown handler. AFAICT the reader mode shortcut (implemented using a <key> element) gets invoked before devtools even runs.

Neil, do you know why, in this case, XUL's <key> element handling happens before devtools, even if focus is in the input box there? Is there anything we can do to fix this?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hey Mossop, I know you did some <key> stuff back in the day for de-XBL'ing... is this something that sounds kinda familiar? In particular, why does the <key> event on a window handle the key events before DevTools, even if input focus is on the devtools and they preventDefault on it? (The <key> looks like it tries to handle the event at the bubbling phase).

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(To be clear, I can reproduce this on Windows.)

Comment 4

[Dave Townsend [:mossop]]

The problem is that devtools aren't calling preventDefault on the event. The handler that Gijs points to is the one on the Search History input itself and that does preventDefault and indeed reader mode doesn't open if you press F9 while that is focused. The handler for the console editor panel though is this one: https://searchfox.org/mozilla-central/source/devtools/client/webconsole/components/App.js#137-148

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Good eye, thanks Mossop!

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Thanks Gijs and Dave for the investigation, I'll fix that

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1682340 - [devtools] Prevent triggering Reader Mode with reverse search keyboard shortcut on Windows. r=bomsy.

We weren't calling preventDefault on the event handler, which was
allowing the browser to consume it, and toggle reader mode on windows,
which shares the same keyboard shortcut (F9).
A test is added to ensure we don't regress this (the test is failing without
the fix).

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65d5af50131e
[devtools] Prevent triggering Reader Mode with reverse search keyboard shortcut on Windows. r=bomsy.

Comment 9

[Alexandru Michis [:malexandru]]

Backed out changeset 65d5af50131e (bug 1682340) for causing dt failures in browser_jsterm_editor_reverse_search_keyboard_navigation.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/7c19b76d5d7d0e1bd5d1311f5c3e5d8a999472d5

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&fromchange=ecd3cad19246552d8c9f5543b6c47f406e635ed3&searchStr=dt%2Cwindows%2C10%2Cx64%2Cdebug&tochange=7c19b76d5d7d0e1bd5d1311f5c3e5d8a999472d5&selectedTaskRun=PvL0gSouQAyqx5OYHR5NJg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=327062358&repo=autoland&lineNumber=40423

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7deb43b486c1
[devtools] Prevent triggering Reader Mode with reverse search keyboard shortcut on Windows. r=bomsy.

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/7deb43b486c1