1283341 - Consider script run from the console as user input

Status: NEW

(DevTools :: Console, enhancement, P3)

Description

[Xidorn Quan [:xidorn] UTC+10]

Currently, if you want to try things like requestFullscree or window.open in the console, you would be blocked because it is not inside handling of a user input event.

But inputing to console and press enter there is actually a user input, right? It doesn't make sense to block invoking those API there.

I propose to consider script run directly from console as user input, just like click, keypress, etc.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

We have a function wrapHandlingUserInput in E10SUtils [1] for wrapping code that is treated as handling a user input. It seems devtools is a pretty independent component that never uses E10SUtils, so I have no idea what should be done here. Hopefully someone from the devtools team could help.

[1] https://dxr.mozilla.org/mozilla-central/rev/b69a5bbb5e40bd426e35222baa600b481e50d265/browser/modules/E10SUtils.jsm#116-126

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

FWIW, in Chrome, you can call requestFullscreen to enter fullscreen directly from the console, so I suppose they already have this.

Comment 3

[Brian Grinstead [:bgrins]]

Code executed in the console is run through the Debugger API via frame.evalWithBindings [1] or dbgWindow.executeInGlobalWithBindings [2] depending on whether you are paused or not.

Maybe wrapping this in the call outlined in Comment 1 would 'automatically' make this work?  Though I'm concerned about the try / finally needed there - is setHandlingUserInput expected to throw?

[0]: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/webconsole.js#1295
[1]: https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Frame
[2]: https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Object

Comment 4

[Brian Grinstead [:bgrins]]

As an alternative, we should consider making anything evaluated through Debugger API act this way instead of just the console.  I don't know enough about the full implications of 'user input' to know if that's a good idea - Xidorn, maybe you can help explain that - but it would be more complete than special casing a particular evaluate method in the console actor.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

The JS engine knows nothing about user inputs (that's all DOM code) and trying to shim it into the Debugger API will be painful.

Instead, I think the actor eval methods should call the same setHandlingUserInput method that helper does[0]. It doesn't make sense to use that helper directly since we don't have a function to wrap.

[0] https://dxr.mozilla.org/mozilla-central/rev/b69a5bbb5e40bd426e35222baa600b481e50d265/browser/modules/E10SUtils.jsm#119-121

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] from comment #5)
> The JS engine knows nothing about user inputs (that's all DOM code) and
> trying to shim it into the Debugger API will be painful.
> 
> Instead, I think the actor eval methods should call the same
> setHandlingUserInput method that helper does[0]. It doesn't make sense to
> use that helper directly since we don't have a function to wrap.
> 
> [0]
> https://dxr.mozilla.org/mozilla-central/rev/
> b69a5bbb5e40bd426e35222baa600b481e50d265/browser/modules/E10SUtils.jsm#119-
> 121

That makes sense, thanks

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> Maybe wrapping this in the call outlined in Comment 1 would 'automatically'
> make this work?  Though I'm concerned about the try / finally needed there -
> is setHandlingUserInput expected to throw?

It would work.

setHandlingUserInput is not expected to throw, but it is safer to wrap it in try / finally block so that in case some code after setHandlingUserInput throws, the user input flag can still be reset properly later. If that flag is not reset properly, content would be able to show popup, enter fullscreen, lock user's pointer whenever they like, which we want to avoid at all cost.

Comment 8

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)