qutebrowser / qutebrowser

A keyboard-driven, vim-like browser based on Python and Qt.
https://www.qutebrowser.org/
GNU General Public License v3.0
9.68k stars 1.01k forks source link

No handling of infinite window.alert loops #3821

Open CharlesSchimmel opened 6 years ago

CharlesSchimmel commented 6 years ago

Using version 1.2.1 but probably version agnostic.

Reproduceable by opening a .html with <script> while(true){ window.alert("alert lock") } </script> (warning: you will have to close the window to escape this.)

Current behavior: javascript alert popup is focused over browser, user cannot exit tab or prevent further popups.

Possible solution: add option to prevent further alerts.

jgkamat commented 6 years ago

Would you think a default bind to close the tab, or a new command to mute js prompts on that page temporarily would be better?

CharlesSchimmel commented 6 years ago

Ideally, I think a holistic option would be to allow the user to exit prompt mode as well as prevent further popups. It seems that in order to focus the dialog, it locks the user's input to the dialog. I don't know about other dialog types, but the download prompt doesn't do this.

The-Compiler commented 6 years ago

FWIW what you can do is middle-click the tab (there's the tabs.close_mouse_button setting, too). I agree something like Chromium's "prevent this page from opening more dialogs" would be a good idea.

chaorace commented 3 years ago

Something funny I encountered which is related to this bug: an infinite window.alert loop can actually crash qutebrowser if you accept too many of them before closing the tab. In the event that such a crash happens, it will summon the crash reporter (good!), which will then be modal blocked by the somehow still-ongoing infinite alert loop (bad!). This seems to render the reporter unusable and I was left with no choice but to force-close the application.

This seems like a two-parter...

1611777382

Version Info:

qutebrowser v1.14.1
Git commit: 21b20116f on master (2021-01-19 19:14:55 +0100)
Backend: QtWebEngine (Chromium 83.0.4103.122)
Qt: 5.15.2
amacfie commented 3 years ago

We could have a modals_disabled field in browser.webengine.webview.WebEnginePage that gets reset in acceptNavigationRequest, but I wonder how other browsers do it because if we're automatically aborting the dialog, while(true){window.alert()} is going to use up 100% CPU and become unresponsive. Closing the tab should be straightforward, just emit windowCloseRequested from browser.webengine.webview.WebEnginePage. Or maybe emit navigation_request to go to url.default_page.

andrewcarlotti commented 3 years ago

This seems like a two-parter...

  • Where are the messages coming from post-crash? Is it possible that the JS context somehow survived the main process crashing?

JavaScript is run in the per-tab render process (a QtWebEngineProcess), so wouldn't be directly affected by a crash in the main Python process. (The number of render processes is capped, so it isn't actually per-tab if you have too many tabs open.)

  • What weirdness is leading to the prompt messages somehow getting redirected to these popups? Maybe there's an old fallback code path somewhere getting activated?

It probably isn't an "old" fallback path, but rather a default path in QtWebEngine that would normally be overridden by Qutebrowser (though I currently can't quite see how that would work).

Mallchad commented 3 years ago

Hi, I just wanted to say that I think one of the reasons this was considered an issue is because the prompt blocks all ability to use bindings. which is understandable in vim land, but practically not useful and gets you stuck in these prompt loops, which are already annoying to begin with on any browser, but worse when you can't close the window or something to stop it. Like mentioned here #6433

I think a nice work around would be to have be a few modified backed keys to prompt mode by default.

For me this looked like a bunch of keybindings Small Extracts:

config.bind("<alt+right>",     "forward", mode="prompt")
# emacs-like
config.bind("<alt-x>",         "set-cmd-text :", mode="prompt")
# misc
config.bind("<ctrl+shift+x>",  "quit --save", mode="prompt")
config.bind("<alt+right>",     "forward", mode="prompt")
# emacs-like
config.bind("<alt-x>",         "set-cmd-text :", mode="prompt")
# misc
config.bind("<ctrl+shift+x>",  "quit --save", mode="prompt")

# yesno
config.bind("<alt+right>",     "forward", mode="yesno")
# emacs-like
config.bind("<alt-x>",         "set-cmd-text :", mode="yesno")
# misc
config.bind("<ctrl+shift+x>",  "quit --save", mode="yesno")
config.bind("<alt+right>",     "forward", mode="yesno")
# emacs-like
config.bind("<alt-x>",         "set-cmd-text :", mode="yesno")
# misc
config.bind("<ctrl+shift+x>",  "quit --save", mode="yesno")

I have not been able to verify how this holds up to prompt spam, but I think at the very least being able to close the tab or qutebrowser should be helpful namely-

config.bind("<ctrl-q>",       "quit", mode="prompt")
config.bind("<ctrl-q>",       "quit", mode="yesno")
config.bind("<ctrl-w>",       "tab-close", mode="prompt")
config.bind("<ctrl-w>",       "tab-close", mode="yesno")

Side note. Considering the amount of keybinds I have personally done this too, it would be immensely useful to be able to specify multiple modes.

I understand if its unfeasible right now :) thanks for making Qutebrowser.

The-Compiler commented 3 years ago

Considering the amount of keybinds I have personally done this too, it would be immensely useful to be able to specify multiple modes.

The config.py is a Python file, so you can e.g. do something like (untested):

def bind_multiple(key, command):
    for mode in ["prompt", "yesno"]:
        config.bind(key, command, mode=mode)
Mallchad commented 3 years ago

Thank you this is exactly what I wanted, never thought to treat the config as code xD