webui: Prevent the Anaconda window from being closed by keyboard shortcuts #4948
Conversation
adamkankovsky
force-pushed
the
webui-prevent-window-to-be-closed
branch
2 times, most recently
from
479615b
to
29dfb98
Compare
There was a problem hiding this comment.
This gives the default dialog from firefox.
@garrett can we keep this or we need a custom dialog?
Also @adamkankovsky can we please have a test for this?
Unfortunately, I think we're stuck with this. You used to be able to customize it, but all browsers removed support for this a few years ago: https://stackoverflow.com/questions/38879742/is-it-possible-to-display-a-custom-message-in-the-beforeunload-popup
We want it specifically for closing the application window, including Ctrl-W, Ctrl-Q, and the (X) icon in the top-right. Bummer. We do not want this prompt when quitting from the UI. That can and should be handled in-page with a PatternFly modal. So does that mean this PR doesn't do what we want it to? |
|
There are apparently a bunch of custom settings one can toggle in Firefox to adjust prompting on closing: https://www.reddit.com/r/firefox/comments/si9in4/comment/hv8hd7v/?context=3 — I can add some of these to the Firefox theme's |
|
@KKoukiou What did you do to get this dialog? |
Ctrl + w. I just tested on the remote accessed browser window. |
|
I tried closing a Firefox window with just one tab... Penpot... and got this modal:
So this should theoretically work for Anaconda too, for the (X) icon. (This is Firefox; I'm just using a custom theme that makes it look more like GNOME.) |
|
@garrett Thanks to the fact that it is done via window.onbeforeunload which captures all window closures from the user side. I've tried it and even on the x button it works. |
🎉 Excellent to hear! |
adamkankovsky
force-pushed
the
webui-prevent-window-to-be-closed
branch
from
253184a
to
901f4a4
Compare
KKoukiou
force-pushed
the
webui-prevent-window-to-be-closed
branch
from
901f4a4
to
c069fb9
Compare
|
Hi. Any reason this hasn't been merged yet? If there's anything that needs to be done still, can we merge this and split whatever it might be out to a follow-up? Thanks! |
so I havent managed to get in the required test changes cockpit-project/cockpit#19148 You can remove the tests - merge it and create a new PR - mark it as blocked on my cockpit PR |
adamkankovsky
force-pushed
the
webui-prevent-window-to-be-closed
branch
from
c069fb9
to
797963e
Compare
adamkankovsky
force-pushed
the
webui-prevent-window-to-be-closed
branch
from
797963e
to
803a982
Compare
adamkankovsky
force-pushed
the
webui-prevent-window-to-be-closed
branch
from
803a982
to
bdc0d85
Compare
…tcuts This commit also removes the b.reload check on the language test, as now reloading the browser will be guarded with a prompt, and we currently dont have a way to reply to that. [1] [1] cockpit-project/cockpit#19148
KKoukiou
force-pushed
the
webui-prevent-window-to-be-closed
branch
from
5e8a31d
to
e1ec43f
Compare
|
/kickstart-test --waive webui-only |
The changes in this PR do not prevent closing the application window, but only add a confirmation window before quitting.
In most browsers this behavior is prevented because it could be used to attack https://stackoverflow.com/questions/30781070/prevent-onbeforeunload-to-close-page-in-any-case.
Therefore, this seems to me to be the only possible solution at the moment.