1540249 - Cmd+R should refresh when the page is paused

Status: NEW

(DevTools :: Debugger, enhancement, P3)

Description

[:Harald Kirschner :digitarald]

What were you doing?

1. Pause in any page
2. Focus the page
3. Refresh via shortcut (Ctrl+R, Cmd-Shift-R, F5, etc)

What happened?

Command ignored. You have to focus devtools or Firefox UI for the shortcut to work.

What should have happened?

Refresh. Works in Chrome.

Raised priority as refresh is a major escape hook for debugging flows and to test updated code.

Comment 1

[Jason Laster [:jlast]]

here are some links for reloading:

https://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.js#412
https://searchfox.org/mozilla-central/source/browser/base/content/browser.js#3433
https://searchfox.org/mozilla-central/source/devtools/client/webreplay/menu.js#33

Comment 2

[Jason Laster [:jlast]]

This might be some low hanging fruit

Comment 3

[malikanshul29]

I and Logan did little digging into this and found that it's probably because we pause, we suppress all content event handling, so the keystrokes never gets handled. https://searchfox.org/mozilla-central/source/devtools/server/actors/targets/browsing-context.js#1195
@jimb I don't know where to go from here.

Comment 4

[Jim Blandy :jimb]

That's surprising, because ctrl-R is not being handled by the page itself, it's supposed to be handled by the browser. We do need to prevent content event handlers from running, but that shouldn't be affecting browser-wide bindings.

I think we'll need to talk to some platform folks about that. Leaving needinfo?me since I'm on the hook to find the right person to consult.

Comment 5

[Jim Blandy :jimb]

Mossop, can you recommend an approach here? I'm just not familiar with event processing at this level at all.

Comment 6

[Dave Townsend [:mossop]]

Can someone clarify what pausing actually does?

Comment 7

[Dave Townsend [:mossop]]

Dropping this needinfo until there is more information.

Comment 8

[Logan Smyth [:loganfsmyth]]

Pausing enters a nested event loop and runs suppressEventHandling(true) and suspendTimeouts() because it is important that the nested event loop continue executing the devtools server itself without processing any events that may be generated by interaction with the page or the user's own code evaluation.

Unfortunately it seems that disabling this handling for the document also has the side-effect of disabling other browser shortcuts like refresh.

Comment 9

[Dave Townsend [:mossop]]

So I think for this keyboard event we first send it to the content process to see if it gets handled there. If not the main process handles it. So it's probably attempting to fire the event on the content's focused element before processing it in the main process. If suppressEventHandling means the event waits to be fired until the document is unpaused then that will be the problem.

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I can still reproduce this bug, should be fixed, but changing to P3