Closed Bug 1766636 Opened 3 years ago Closed 2 years ago

chrome windows opened by private browsing window use regular Firefox taskbar icon

Categories

(Firefox :: Shell Integration, enhancement, P3)

Desktop
Windows
enhancement

Tracking

()

VERIFIED FIXED
105 Branch
Tracking Status
firefox105 --- verified

People

(Reporter: bhearsum, Assigned: bhearsum)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidedi-pbm])

Attachments

(4 files)

Attached video two-icons.mkv

With browser.privacySegmentation.enabled set to true we segregate Private and non-Private browsing windows to their own Taskbar icons. This works well, but anytime a chrome window opens (eg: Page Info) it will always show under the non-Private icon. (Screencast attached.)

In scenarios where there was already a non-Private window open this is not too jarring, but if you only have Private windows open it causes a new Taskbar icon to show up. It seems quite clear that in some of these cases, like Page Info, the chrome window should associate with the Taskbar Icon of the window that it was opened with. In other cases (eg: Browser Console or a detached Developer Tools) the answer seems a bit less obvious to me.

There's two first steps here I'd like to take:

  • Make a list of all of the chrome windows (so we can decide what behaviour is preferable for each)
  • Do some digging into code to see how difficult it would be to get chrome windows showing under the Private icon. I have a feeling that not all of the code that creates a new window will be able to know whether or not the current window is Private or not (but let's find out!)
Severity: -- → S3
Priority: -- → P3
Whiteboard: [fidedi-pbm]
Assignee: nobody → bhearsum

I dug around the primary UI today and found only a handful of chrome windows that we potentially to worry about (there may be others - but this is an easy starting point):

  • Page Info -- If we're to continue showing this in the taskbar this should almost certainly use the same as the window that opens it -- but I wonder if we should stop showing it in the taskbar altogether as we do for things like "About Firefox"?
  • Library -- This should probably also use the same taskbar icon as the opening window (otherwise you end up with an extra taskbar icon if you open it from a private window with no non-private windows)
  • Profile Manager -- This only exists before a browsing window opens, which means there's no "parent" window to instruct us on how to group.
  • Some developer tools (Browser Console when torn away, Browser Toolbox, Browser Content Toolbox) -- These are debatable IMO, especially the Browser Toolbox (which is essentially global to the entire running instance)

I'm currently trying a few things to see if there's a way that we can pull or inherit the AUMID of the opening window somehow - but no luck so far. The parent passed to nsWindow::Create is null in at least one case - so that doesn't work. I also thought that I could get the WinTaskbar code to do it (which already retrieves AUMID) -- but I suspect this doesn't work because we're already in a new process by the time it runs via nsWindow's constructor.

(In reply to bhearsum@mozilla.com (:bhearsum) from comment #1)

I'm currently trying a few things to see if there's a way that we can pull or inherit the AUMID of the opening window somehow - but no luck so far. The parent passed to nsWindow::Create is null in at least one case - so that doesn't work. I also thought that I could get the WinTaskbar code to do it (which already retrieves AUMID) -- but I suspect this doesn't work because we're already in a new process by the time it runs via nsWindow's constructor.

For posterity, I tried very hard to get these approaches to work, but neither panned out. It was possible to get the nsWindow parent set by making them CHROME_DEPENDENT -- but unfortunately, this also implies that we don't want a Taskbar icon at all.

Letting WinTaskbar deal with it doesn't work for the reason I suspected -- there isn't an AUMID set in the process when nsWindow needs it. (There may also be cases where we end up re-using the same process that would cause issues here -- but I didn't bother diving that deep after discovering the aforementioned issue.)

nsWindow::Create will do the right thing (including keeping this behaviour off by default for now) as long as private is set appropriately.

I believe this covers everything except the Browser Content Toolbox (which I believe we're planning to retire?).

Depends on D152460

Attachment #9286528 - Attachment description: Bug 1766636: ensure most devtools Chrome windows opened by a private browsing window group with the correct taskbar icon r?#devtools-reviewers! → Bug 1766636: ensure most devtools Chrome windows opened by a private browsing window group with the correct taskbar icon r?nchevobbe!,jdescottes!
Attachment #9286527 - Attachment description: Bug 1766636: ensure places organizer and page info dialog opened by private browsing window group with the correct taskbar icon r?gijs! → Bug 1766636: ensure page info dialog opened by private browsing window groups with the correct taskbar icon r?gijs!,mak!
Attachment #9286528 - Attachment description: Bug 1766636: ensure most devtools Chrome windows opened by a private browsing window group with the correct taskbar icon r?nchevobbe!,jdescottes! → Bug 1766636: ensure Browser Content Toolbox groups with the correct Windows taskbar icon r?jdescottes!
Attachment #9286527 - Attachment description: Bug 1766636: ensure page info dialog opened by private browsing window groups with the correct taskbar icon r?gijs!,mak! → Bug 1766636: ensure page info dialog opened by private browsing window groups with the correct taskbar icon r?gijs!
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ae650f88741f ensure page info dialog opened by private browsing window groups with the correct taskbar icon r=Gijs https://hg.mozilla.org/integration/autoland/rev/886b1d4088b5 ensure Browser Content Toolbox groups with the correct Windows taskbar icon r=nchevobbe,jdescottes https://hg.mozilla.org/integration/autoland/rev/829146705615 set private window feature correctly for profile manager window, to ensure they use the correct windows taskbar icon r=mossop

Hi Ben! I've checked these patches on Windows 10 x64 with latest Nightly 105.0a1, and it seems that some dialogs are still not opened under PBM taskbar icon:

  • Browser toolbox not opened under PBM taskbar icon, see screencast.
  • Browser console not opened under PBM taskbar icon, see screencast.
  • Library not opened under PBM taskbar icon, see screencast.
  • Profile Manager Dialog when opened from terminal by using -private parameter, see screencast.

If I understand correctly from the above comments, certain dialogs are supposed to open under the regular Firefox icon. Can you please take a look at the above, and let us know your thoughts?

Flags: needinfo?(bhearsum)

(In reply to Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA from comment #8)

Hi Ben! I've checked these patches on Windows 10 x64 with latest Nightly 105.0a1, and it seems that some dialogs are still not opened under PBM taskbar icon:

Apologies, we decided against doing this in review, but I failed to update the bug :(. These are all expected behaviour at this point.

  • Profile Manager Dialog when opened from terminal by using -private parameter, see screencast.

-private is not a valid command line option - you want -private-window.

Flags: needinfo?(bhearsum) → needinfo?(ciprian.georgiu)

Confirming that we're seeing the expected behavior after chatting a bit with Ben on slack. Thanks!

Status: RESOLVED → VERIFIED
Flags: needinfo?(ciprian.georgiu)

And just to echo what I said on Slack: the Profile Manager will not show up with the private icon until we make this the default behaviour (because there's no way to override the default value of browser.privacySegmentation.windowSeparation.enabled before we have a profile). Once that's changed, -P -private-window will use the private icon (a scenario I tested by doing a custom build where this is the default behaviour).

Depends on: 1789227
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: