Closed Bug 1735748 Opened 3 years ago Closed 2 years ago

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

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(firefox105 verified)

VERIFIED FIXED
105 Branch
Tracking Status
firefox105 --- verified

People

(Reporter: whimboo, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.

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

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.

Severity: -- → S3
Priority: -- → P3

Alex, is anything blocking us from landing the patch?

Flags: needinfo?(poirot.alex)

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 :)

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?

Flags: needinfo?(standard8)

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.

Flags: needinfo?(poirot.alex)

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.

Flags: needinfo?(standard8)

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.

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

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.

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 ?)

Attachment #9245921 - Attachment description: Bug 1735748 - [devtools] Restart the whole browser instead of only reloading the topmost window in the browser toolbox. → Bug 1735748 - [devtools] Disable all DevTools reload shortcuts in the Browser Toolbox, but replicate the local-build-only full reload shortcut.

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

Pushed by apoirot@mozilla.com: 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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
QA Whiteboard: [qa-105b-p2]

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.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-105b-p2]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: