1735748 - Cmd/Ctrl+R in Multiprocess Browser Toolbox blanks the whole browser window

Status: VERIFIED FIXED

(DevTools :: General, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:95.0) Gecko/20100101 Firefox/95.0 ID:20211011214613

Hitting Cmd/Ctrl+R from within the Multiprocess Browser Toolbox the whole underlying browser window gets reloaded, and ends-up totally blank. All open tabs are gone. Given that I don't know if it could end-up in dataloss I force-killed Firefox to not let it write the sessionstore data.

Maybe we have to disallow reloading? Or at least bring back the browser window to a state with all usual content displayed.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I just tested and after a normal quit and restart of the browser all the former tabs are getting loaded again.

Comment 2

[Julian Descottes [:jdescottes]]

Instead of reloading the topmost window as we do today, we should perform the same thing as what happens when you use the "Restart (Developer)" feature from local builds.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1735748 - [devtools] Disable all DevTools reload shortcuts in the Browser Toolbox, but replicate the local-build-only full reload shortcut.

Comment 4

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

Alex, is anything blocking us from landing the patch?

Comment 5

[Julian Descottes [:jdescottes]]

FWIW, I don't think we had a strong consensus on what this patch was doing. Reloading the whole browser is quite disruptive even if it's better than blanking it. We were hesitant between

• reloading the browser (the attached patch)
• ignoring the shortcut when used from the Browser Toolbox

So I think this bug needs a discussion + decision to move forward :)

Comment 6

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

I see, thank you Julian.

I tend to say that we should reload the Browser UI (From Alex in phab: If you are actively working on Firefox and want to reload it to pick up your changes, it is as important as doing F5 in the web page workflow)

Mark, do you have a proffered option here?

Comment 7

[Alexandre Poirot [:ochameau]]

Mark and I pinged the crowd on matrix #developers/#Firefox Desktop Development without much feedback.

It highlights how hard it is to get feedback about the browser devtools.
Thanks to the survey, we know we have a few users and they have opinions about our tools!
So we might try to find other ways to reach our users...

It may also tell us that's an edgecase and not a very important item.

Comment 8

[Mark Banner (:standard8)]

As Alexandre said, I don't think we have any major opinions here. For myself, I didn't even know it was an option (nor had I thought about it being an option).

I think as you have a patch for making reload work, then maybe land that & advertise it, and if it starts causing issues for developers, then maybe turn it off.

Comment 9

[Julian Descottes [:jdescottes]]

I was about to say the same thing as Mark :) Let's land this and gather feedback. We can always gate it behind a pref if it is annoying for users. At least we will already have the dedicated codepath to do that.

Comment 10

[Julian Descottes [:jdescottes]]

For the record, we quickly discussed about this during the tools checkin meeting.
There was a suggestion on the google docs for the meeting to reload the browser only on cmd or ctrl + R, and do nothing on F5.
I would suggest to keep the same behavior for both shortcuts: either do nothing or reload the browser. I don't think F5 is more accidental than cmd+R for Browser Toolbox users, and I don't see a good reason to pick one shortcut over the other.

I already mentioned that my preference would be to do nothing for now, especially because it's not discoverable & documented for end users.
But maybe we could leverage the Browser Toolbox toolbar that :nchevobbe is adding? And add a "reload browser" button there? I would be less concerned about cmd+R / F5 reloading the browser if it was mapped to a very visible part of the UI.

Comment 11

[Alexandre Poirot [:ochameau]]

Sorry, I wrote the comment on the google doc too quickly.

I meant to only keep the key shortcut set on local builds, which is designed for Firefox contributors, which should be the audience of the browser toolbox. And I think that's something relevant to see working from the browser toolbox.
https://searchfox.org/mozilla-central/source/browser/base/content/browser-development-helpers.js#35-40

The behavior I was trying to replicate is the exact behavior of this special key shortcut... And I didn't realize until recently that it was different and required "Alt" to be also pressed.

I updated the patch to disable all DevTools reload key shortcuts in the browser toolbox, while replicating the CmdOrCtrl+Alt+R one. ("accel" is CmdOrCtrl on mac, right ?)

Comment 12

[Julian Descottes [:jdescottes]]

Reusing the Alt variant is perfect here, thanks for the update.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dbd81dc3e2d
[devtools] Disable all DevTools reload shortcuts in the Browser Toolbox, but replicate the local-build-only full reload shortcut. r=jdescottes,nchevobbe

Comment 14

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/2dbd81dc3e2d

Comment 15

[Ardelean Oana, Desktop QA [:oardelean]]

I was able to reproduce this issue on a 2022-08-06 Nightly build on macOS 12 using the STR from the Description. Verified as fixed on Firefox 105.0b8(build ID: 20220906185728) and Nightly 106.0a1(build ID: 20220906092849) on macOS 12, Windows 10, Ubuntu 22.04.