Closed Bug 1832331 Opened 1 year ago Closed 1 year ago

Recent PiP changes clash with Microsoft PowerToys layout manager (FancyZones)

Categories

(Toolkit :: Picture-in-Picture, defect, P3)

Product:
Toolkit
Component:
Picture-in-Picture
Version:
Firefox 113
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
115 Branch
Tracking Flags:
Tracking Status
relnote-firefox --- 113+
firefox-esr102 --- unaffected
firefox113 --- verified
firefox114 --- verified
firefox115 --- verified

People

(Reporter: wirelessbrain, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Firefox v112.0 working fine with Microsoft PowerToys FancyZones
9.15 MB, video/x-matroska
Details
Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman
48 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta+
RyanVM
: approval-mozilla-release+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/113.0

Steps to reproduce:

  • Update to Firefox 113.0
  • Use Picture-in-Picture for any type of video media
  • Hold left-click on the PiP window while hitting right-click to activate zone-snapping

Actual results:

FancyZones does not register the PiP window as snappable to the layout and therefore the PiP window can't be assigned to a zone.

Expected results:

Before the PiP window changes, PiP windows worked fine with FancyZones. You hold left-click on the window and hit right-click once (and release). This way you could move the window to a zone of choice and let go of holding left-click.

The Bugbug bot thinks this bug should belong to the 'Toolkit::Picture-in-Picture' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Picture-in-Picture
Product: Firefox → Toolkit

Hi nakami,

Thanks filing this bug. Considering that FancyZones worked with PiP in the past for you, would you be able to provide a regression range to help us determine when the defect was introduced? You can consult our mozregression documentation here: https://mozilla.github.io/mozregression/quickstart.html

Severity: -- → S3
Flags: needinfo?(wirelessbrain)
Priority: -- → P3

Last good build:
app_name: firefox
build_date: 2023-03-21 15:25:43.434000
build_file: C:\Users\nakami.mozilla\mozregression\persist\61e118ecd994-pgo--autoland--target.zip
build_type: integration
build_url: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/N8i7Iv-FRcC3JtkJJiM-2g/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: 61e118ecd994b89ce397e7f838a703eeba049664
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=61e118ecd994b89ce397e7f838a703eeba049664&tochange=fd2a3531261bad2a4de38828e6499e7a8cd7081c
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: N8i7Iv-FRcC3JtkJJiM-2g

First bad build:
app_name: firefox
build_date: 2023-03-21 15:26:46.205000
build_file: C:\Users\nakami.mozilla\mozregression\persist\e1733deee7bd-pgo--autoland--target.zip
build_type: integration
build_url: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/bvxgQPzwRmezoo0tLhwvJw/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: e1733deee7bd41d02613b63176c04433540d391a
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=61e118ecd994b89ce397e7f838a703eeba049664&tochange=e1733deee7bd41d02613b63176c04433540d391a
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: bvxgQPzwRmezoo0tLhwvJw

Flags: needinfo?(wirelessbrain)

Verified my claim by running both builds by downloading and testing the "build_url" zip-archive.

Thanks for the regression details. The pushlog is pointing towards Bug 1823350.

Emilio - could you help us identify what's happening here?

Flags: needinfo?(emilio)

Hmm, so there's something about the titlebar=no that this tool doesn't like. I have an idea after a cursory look at what that might be. I'm not on windows atm so it'll be a tentative patch for now...

Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Regressed by: 1823350

In a bit there should be a build here (click on the green B, when it appears, and in the "Artifacts" tab at the bottom, there should be a target.zip you can extract and run): https://treeherder.mozilla.org/jobs?repo=try&revision=6930a99e358ec8aafe7d753d6c2561d14daea8ad

nakami, it'd be great if you can check if that fixes the issue, because I haven't used that FancyZones tool before, and I can't test windows at least until tomorrow. Leaving ni? open in any case so I can get to test it.

Flags: needinfo?(wirelessbrain)

Set release status flags based on info from the regressing bug 1823350

status-firefox113: --- → affected
status-firefox114: --- → affected
status-firefox115: --- → affected
status-firefox-esr102: --- → unaffected

Yes, that build fixes the issue. Thank you!

Offtopic: Whenever you're using Windows, I highly recommend giving FancyZones a try. It's a component of the Microsoft's Toolkit PowerToys, which comes with many convenient enhancements. FancyZones is basically a Tiling Manager as we know them from Unix OS.

Flags: needinfo?(wirelessbrain)
Assignee: nobody → emilio
Attachment #9332928 - Attachment description: WIP: Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman → Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman
Status: NEW → ASSIGNED
Flags: needinfo?(emilio)

Issue persist not only with FancyZones, but with Win 11 built-in snapping feature: https://support.microsoft.com/en-us/windows/snap-your-windows-885a9b1e-a983-a3b1-16cd-c531795e6241#WindowsVersion=Windows_11

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08997371f09c
Don't force WS_POPUP for windows without a titlebar. r=rkraesig

https://hg.mozilla.org/mozilla-central/rev/08997371f09c

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox115: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 115 Branch

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)

There is a workaround/fix in FancyZones:
"I fixed it by checking the boxes for 'allow popup' and 'child window' snapping in the power toys fancy zones settings."
https://github.com/microsoft/PowerToys/issues/25939#issuecomment-1545274822

However, as n4nn31355 describes, Windows 11's snapping doesn't have this option, I suspect. Cannot test myself as I don't have access to a Windows 11 machine as of now.

Comment on attachment 9332928 [details]
Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman

Beta/Release Uplift Approval Request

  • User impact if declined: comment 0
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Relatively simple change which shouldn't affect other windows (see phab revision as for why).
  • String changes made/needed: none
  • Is Android affected?: No
Flags: needinfo?(emilio)
Attachment #9332928 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9332928 [details]
Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman

Approved for 114 beta 5, thanks.

Attachment #9332928 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is Verified as fixed in our latest Beta 114.0b5 as well as our latest Nightly build 115.0a1 (2023-05-16).

status-firefox114: fixed → verified
status-firefox115: fixed → verified

What do you think about nominating this for next week's planned dot release given this is a new regression in 113? Go ahead and nominate if yes.

Flags: needinfo?(emilio)

I guess since it affects the native win 11 snapping apparently (see comment 16) it might be worth it given it's a relatively simple patch.

Flags: needinfo?(emilio)

Comment on attachment 9332928 [details]
Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman

Beta/Release Uplift Approval Request

  • User impact if declined: comment 0
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): While widget code is always a bit tricky, the fix is very simple, and even if it affected other windows other than PiP (which it really shouldn't given my audit, but...), the consequences would be less bad than the bug, so seems worth uplifting.
  • String changes made/needed: none
  • Is Android affected?: No
Attachment #9332928 - Flags: approval-mozilla-release?

Comment on attachment 9332928 [details]
Bug 1832331 - Don't force WS_POPUP for windows without a titlebar. r=cmartin,rkraesig,handyman

Approved for 113.0.2.

Attachment #9332928 - Flags: approval-mozilla-release? → approval-mozilla-release+

This issue is Verified as fixed in our latest 113.0.2 (64-bit).

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
status-firefox113: fixed → verified
Flags: qe-verify+

Added to the 113.0.2 release notes:

Fixed an issue which caused Picture-in-Picture windows to not be snappable on Windows 11 or on systems with the FancyZones PowerToy installed

relnote-firefox: --- → 113+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: