Closed Bug 1641546 Opened 6 months ago Closed 6 months ago

Global Sharing Overlay's position can't be moved on linux and windows

Categories

(Firefox :: Site Permissions, defect, P1)

78 Branch
Desktop
Unspecified
defect

Tracking

()

VERIFIED FIXED
Firefox 79
Tracking Status
firefox-esr68 --- unaffected
firefox77 --- unaffected
firefox78 --- verified
firefox79 --- verified

People

(Reporter: sergiu.negritas, Assigned: mconley)

References

(Blocks 1 open bug, Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(6 files)

Affected versions

  • 78.0a1

Affected platforms

  • Ubuntu 18.04 LTS

Steps to reproduce

  1. Engage in a videoconference call.
  2. Observe the Global Sharing Overlay.
  3. Move the Global Sharing Overlay on the screen.

Expected result

  • Global Sharing Overlay can be moved anywhere on the screen.

Actual Result

  • The Global Sharing Overlay can't be moved from it's initial position.

Regression range

  • This is a recent regression.

Additional notes

  • This issue occurred after the Global Sharing Overlay title bar was fixed.
Summary: Global Sharing Overlay's position can't be moved → Global Sharing Overlay's position can't be moved on linux
Priority: -- → P1
Duplicate of this bug: 1641240

Setting to P2, as vchin has given me feedback that we can enable the new indicator on Linux in a later release.

Priority: P1 → P2

It appears that this issue is also reproducible in Windows, along with the Linux.

OS: Linux → Unspecified
Summary: Global Sharing Overlay's position can't be moved on linux → Global Sharing Overlay's position can't be moved on linux and windows
Assignee: nobody → mconley
Priority: P2 → P1

This appears to be a regression caused by bug 1639997... the window features that I needed to set in order to get the tests to pass seem to cause this. Starting to investigate.

Regressed by: 1639997

So this is related to the fact that I added "popup=yes" to the feature string for the window. This prevents the indicator from pulling focus, which is necessary to make some tests pass (and is probably a good idea, accessibility-wise). Windows of type "popup", however, appear to have different rules on different OS's. In particular, on Windows and Linux, popup windows aren't draggable via -moz-window-dragging: drag;. They also have different minimizing / closing characteristics.

I think the right solution here is probably to remove the "popup=yes" bit, but also find a way of ensuring that the indicator window does not pull focus. I'm looking into that now.

This has the added benefit of not causing the Picture-in-Picture player window to pull focus when it opens.

Blocks: 1642799
Group: mozilla-employee-confidential
Blocks: 1643010
See Also: → 1640973
See Also: → 1642800
Blocks: 1641533

(In reply to Mike Conley (:mconley) (:⚙️) (Extremely busy) from comment #11)

This has the added benefit of not causing the Picture-in-Picture player window to pull focus when it opens.

That could be a problem for a screen reader user. While the sharing indicator is essentially an alert notification, a user possibly wants to interact with the picture-in-picture window. If a screen reader user activates picture-in-picture, they now won't get any indication that anything happened.

Hey Jamie,

Hm, okay, good point. Would it be sufficient to fire an "AlertActive" event manually for the Picture-in-Picture player window when it opens, like we do here for the WebRTC indicator?: https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/browser/base/content/webrtcIndicator.js#221-228

Flags: needinfo?(jteh)
Blocks: 1643545

(In reply to Mike Conley (:mconley) (:⚙️) (Extremely busy) from comment #13)

Would it be sufficient to fire an "AlertActive" event manually for the Picture-in-Picture player window when it opens, like we do here for the WebRTC indicator?: https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/browser/base/content/webrtcIndicator.js#221-228

I... never even knew about that event. 😳 Screen readers will ignore that unless the element has role="alert". But adding role="alert" to the PIP window feels wrong; it's not a notification.

I'd honestly prefer it continued to get focus, but I guess comment 11 suggests that non-screen reader users don't like this. (I guess for mouse users, focusing it doesn't serve any purpose because they can still interact with the window using the mouse?)

What about the keyboard shortcut? When people activate PIP with the keyboard, does it follow they want to interact with PIP using the keyboard, or is that not what users expect?

If we really don't want this to get focus, the only option I can come up with is to use A11yUtils.announce to tell screen readers something like "Picture-in-Picture window opened in the background." But I really don't like it.

Marco, any better ideas?

Flags: needinfo?(jteh) → needinfo?(mzehe)

Eek, no, because if an NVDA user activates the PIP window through the virtual cursor, it will simulate a mouse click, not a keyboard event. And most NVDA users will do it from the virtual buffer. I know I would. I wouldn't change fingers just to press the PIP keyboard shortcut. So if this never gets focus any more, an announcement is the only way we have to let screen readers know that this PIP window appeared. The old problem of focus having to be in more places than one, visual parallel processing vs. sequential speech/keyboard processing.

And no, we definitely don't want the PIP window to be an alert. Interacting with that would become cumbersome I think. Although it might be worth experimenting with it just to see if this assumption is accurate. But usual experience suggests it is.

Flags: needinfo?(mzehe)

Okay, thanks Jamie and MarcoZ. I guess what I'll do is land a separate patch in the PiP front-end code to make sure that the player window gets focus automatically upon opening to persist the old behaviour.

The other patches in this stack made it so that alwaysontop windows don't focus by
default when they open. We do, however, want the Picture-in-Picture player window to
take focus after it opens.

See Also: → 1643707
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/041cd14e3893
Make sure alwaysontop windows don't pull focus when first opening on macOS. r=mstange
https://hg.mozilla.org/integration/autoland/rev/2dc2a430c2e7
Make sure alwaysontop windows don't pull focus when first opening on Windows. r=mhowell
https://hg.mozilla.org/integration/autoland/rev/a2b64d290c0b
Make sure alwaysontop windows don't pull focus when first opening on Linux GTK. r=stransky
https://hg.mozilla.org/integration/autoland/rev/4746c37be3e6
Add a regression test for alwaysontop windows pulling focus. r=mstange
https://hg.mozilla.org/integration/autoland/rev/72a62a455c47
Make the new WebRTC global sharing indicator a dialog instead of a popup. r=pbz
https://hg.mozilla.org/integration/autoland/rev/2652e1c084a7
Focus the Picture-in-Picture player after it opens. r=mstriemer
Regressions: 1334752
Status: RESOLVED → REOPENED
Flags: needinfo?(mconley)
Resolution: FIXED → ---
Target Milestone: Firefox 79 → ---

Looking.

Flags: needinfo?(mconley)
Attachment #9153204 - Attachment description: Bug 1641546 - Make sure alwaysontop windows don't pull focus when first opening on Linux GTK. r?mstransky → Bug 1641546 - Make sure alwaysontop windows don't pull focus when first opening on Linux GTK. r?stransky
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b98bd961f10
Make sure alwaysontop windows don't pull focus when first opening on macOS. r=mstange
https://hg.mozilla.org/integration/autoland/rev/abb3f715d112
Make sure alwaysontop windows don't pull focus when first opening on Windows. r=mhowell
https://hg.mozilla.org/integration/autoland/rev/225dfac02102
Make sure alwaysontop windows don't pull focus when first opening on Linux GTK. r=stransky
https://hg.mozilla.org/integration/autoland/rev/c4aa2a4350d2
Add a regression test for alwaysontop windows pulling focus. r=mstange
https://hg.mozilla.org/integration/autoland/rev/85d2124732a3
Make the new WebRTC global sharing indicator a dialog instead of a popup. r=pbz
https://hg.mozilla.org/integration/autoland/rev/8d51c9b612ec
Focus the Picture-in-Picture player after it opens. r=mstriemer

Comment on attachment 9152803 [details]
Bug 1641546 - Make sure alwaysontop windows don't pull focus when first opening on macOS. r?mstange!

Beta/Release Uplift Approval Request

  • User impact if declined: An experiment to use the new WebRTC indicator will run with a semi-broken indicator.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1642800
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The riskiest part is the change to the widget layer, but that's only for alwaysontop windows, for which there are only two kinds: the Picture-in-Picture player window (which is heavily tested in automation) and the new indicator (which is off by default).
  • String changes made/needed: None.
Attachment #9152803 - Flags: approval-mozilla-beta?
Attachment #9152804 - Flags: approval-mozilla-beta?
Attachment #9152847 - Flags: approval-mozilla-beta?
Attachment #9153204 - Flags: approval-mozilla-beta?
Attachment #9153205 - Flags: approval-mozilla-beta?
Attachment #9154576 - Flags: approval-mozilla-beta?

Comment on attachment 9152803 [details]
Bug 1641546 - Make sure alwaysontop windows don't pull focus when first opening on macOS. r?mstange!

approved for 78.0b6

Attachment #9152803 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9152804 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9152847 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9153204 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9153205 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9154576 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is now fixed in Nightly v79.0a1 and Beta v78.0b9. It does not occur on any of the main OSes. Thank you.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.