1791757 - PiP windows opened by private browsing window use regular Firefox taskbar icon

Status: VERIFIED FIXED

(Firefox :: Shell Integration, enhancement, P1)

Description

[Fanolian]

Attachment: PiP with Private Window Separation.png

+++ This bug was initially created as a clone of Bug #1766636 +++

With browser.privacySegmentation.enabled browser.privateWindowSeparation.enabled set to true we segregate Private and non-Private browsing windows to their own Taskbar icons. This works well, but anytime a chrome Picture-in-Picture window opens it will always show under the non-Private icon.

Comment 1

[Fanolian]

[Tracking Requested - why for this release]:
The "private" sense of video playing is paramount and PiP is a common interface general users use. It seems that Private Window Separation will be enabled by default[1] in Firefox 106? I think this bug should be tracked before causing confusions to general users.

[1]: Firefox 105 has this feature in a different parameter browser.privacySegmentation.windowSeparation.enabled but it is disabled by default.

Comment 2

[[e-mail] (:bhearsum)]

Thanks for filing, clearly an oversight from earlier work.

Comment 3

[[e-mail] (:bhearsum)]

Attachment: Bug 1791757: PiP windows opened by private browsing window use regular Firefox taskbar icon r?mconley!

I didn't realize that PiP windows had taskbar icons when I dealt with the other chrome windows in https://bugzilla.mozilla.org/show_bug.cgi?id=1766636 -- the fix is essentially the same.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/981c8a605999
PiP windows opened by private browsing window use regular Firefox taskbar icon r=mconley

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/981c8a605999

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 9295619 [details]
Bug 1791757: PiP windows opened by private browsing window use regular Firefox taskbar icon r?mconley!

Beta/Release Uplift Approval Request

• User impact if declined: PiP windows opened from a Private Browsing window will open under the regular Firefox taskbar icon - causing confusion about whether or not their viewing is actually happening under Private Browsing
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1) Open a Private Browsing window

2. Pop out a PiP window

Expected result: PiP window should show up under the Private icon

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Covered by automated tests; fix is more or less identical to similar fixes for other window types that have already been verified.
• String changes made/needed:
• Is Android affected?: No

Comment 7

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9295619 [details]
Bug 1791757: PiP windows opened by private browsing window use regular Firefox taskbar icon r?mconley!

Approved for 106.0b4, thanks.

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b27103b41072

Comment 9

[Daniel Bodea [:danibodea]]

This issue was reproduced in Release v105.0.1 (using pref browser.privacySegmentation.windowSeparation.enabled) and in Beta v106.0b3 (where browser.privateWindowSeparation.enabled is true by default) and verified the fix in Nightly v107.0a1 and Beta v106.0b4 taken from treeherder.
Testing was done on Windows 10.

Comment 10

[Fanolian]

Attachment: Bug 1791757 after fix D157849.png

This is how it looks on my Windows 10 from build 20220923093059. (and Windows 11 on my another machine)
The PiP window is correctly grouped with other private windows but showing the wrong icon.

:danibodea is it the same for you? Thanks.

Comment 11

[[e-mail] (:bhearsum)]

(In reply to Fanolian from comment #10)

Created attachment 9295993 [details]
Bug 1791757 after fix D157849.png

This is how it looks on my Windows 10 from build 20220923093059. (and Windows 11 on my another machine)
The PiP window is correctly grouped with other private windows but showing the wrong icon.

:danibodea is it the same for you? Thanks.

Can you please clarify what is going on in this screenshot? It looks to me like your icons are not grouped at all - I see 4 separate taskbar groups/icons in it. Are there some non-default Windows prefs at play here?

Comment 12

[Fanolian]

First Nightly icon (title: Firefox Nightly First...): Non-private window
It is separated with other private windows, opened in the same instance as the non-private one, by another software (Calculator).

The next 3 Nightly icons were in the same group. I focused the PiP window so it has a different colour. Sorry for the confusion.
2nd icon (title: Podcast): the private window the PiP window is popped out from.
3rd: the PiP window in question with a non-private icon.
4th (Page info): Page info of the private window. I included this in the screenshot to show that PiP window with an incorrect icon would look weird when sandwiched between other private icons in the same group.

(In reply to [e-mail] (:bhearsum) from comment #11)

Are there some non-default Windows prefs at play here?

I think the only relevant change is Windows 10 Setting > Personalisation > Taskbar > Combine taskbar buttons > When the taskbar is full or Never.
If I select Always hide labels, the bug is less obvious because the incorrect icon can only be seen in the Taskbar Thumbnail Preview when I hover on the group. (Windows 11 always hides labels. It can't be changed. This issue is on Windows 11 too.)

The issue is also exhibited on another computer which never has installed Nightly. Thumbnail cache should not be an issue.
I also asked in a forum. One said they saw the same issue.

Comment 13

[[e-mail] (:bhearsum)]

(In reply to Fanolian from comment #12)

First Nightly icon (title: Firefox Nightly First...): Non-private window
It is separated with other private windows, opened in the same instance as the non-private one, by another software (Calculator).

The next 3 Nightly icons were in the same group. I focused the PiP window so it has a different colour. Sorry for the confusion.
2nd icon (title: Podcast): the private window the PiP window is popped out from.
3rd: the PiP window in question with a non-private icon.
4th (Page info): Page info of the private window. I included this in the screenshot to show that PiP window with an incorrect icon would look weird when sandwiched between other private icons in the same group.

(In reply to [e-mail] (:bhearsum) from comment #11)

Are there some non-default Windows prefs at play here?

I think the only relevant change is Windows 10 Setting > Personalisation > Taskbar > Combine taskbar buttons > When the taskbar is full or Never.
If I select Always hide labels, the bug is less obvious because the incorrect icon can only be seen in the Taskbar Thumbnail Preview when I hover on the group. (Windows 11 always hides labels. It can't be changed. This issue is on Windows 11 too.)

The issue is also exhibited on another computer which never has installed Nightly. Thumbnail cache should not be an issue.
I also asked in a forum. One said they saw the same issue.

Thanks for the clarifications! It seems like the code that sets the window icon is not getting run for some reason (even though the code that sets the AUMID seems to be). I'll do some more digging here.

Comment 14

[[e-mail] (:bhearsum)]

Attachment: Bug 1791757: don't override PiP window icon for private browsing windows r?mconley!

nsWindow::Create already sets this icon correctly for all Private Browsing windows (even dialog ones) - we need to make sure we're not overriding it.

I thought about tests for this, but ultimately the only change effected here is to a Windows HWND that we can't really verify anything on. We could (in theory anyways) mock out the call to setWindowIconNoData and check whether it was or was not called -- but I'm not sure if we typically do that for this sort of thing. I'd appreciate some guidance on this part.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27538ef74557
don't override PiP window icon for private browsing windows r=mconley

Comment 16

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/27538ef74557

Comment 17

[Fanolian]

(In reply to [e-mail] (:bhearsum) from comment #14)

Created attachment 9296246 [details]
Bug 1791757: don't override PiP window icon for private browsing windows r?mconley!

nsWindow::Create already sets this icon correctly for all Private Browsing windows (even dialog ones) - we need to make sure we're not overriding it.

It is fixed for me in 20221004213513. Thank you.

Comment 18

[[e-mail] (:bhearsum)]

Comment on attachment 9296246 [details]
Bug 1791757: don't override PiP window icon for private browsing windows r?mconley!

Beta/Release Uplift Approval Request

• User impact if declined: PiP windows opened from a Private Browsing window will appear to be not private.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It is a very targeted change that just aborts a code path that improperly overrides window grouping settings for Private windows.
• String changes made/needed:
• Is Android affected?: No

Comment 19

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9296246 [details]
Bug 1791757: don't override PiP window icon for private browsing windows r?mconley!

Approved for 106.0b9 (last beta before our release candidate). Also please don't add patches to closed bugs bur open a follow up bug instead, thanks!

Comment 20

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0de8de679666