1640973 - Global Sharing Overlay has a title bar with a close option

Status: VERIFIED FIXED

(Firefox :: Site Permissions, defect, P1)

Description

[Negritas Sergiu, Desktop QA]

Attachment: GlobalSharingOverlay.png

Affected versions

• 78.0a1(2020-05-26)

Affected platforms

• Ubuntu 18.04 LTS

Steps to reproduce

1. Engage in a video conference call (talky.io)
2. Click on the share screen button
3. Select a screen option from the "Select Window or Screen to share" drop-down and click on the "Allow" button.
4. Observe the Global Sharing Overlay.

Expected result

• Global Sharing Overlay should not have a title bar with a close button as seen in the image attached.

Actual Result

• Global Sharing Overlay has a title bar with a close button above it and you can close it without being able to bring it back.

Regression range

• This is a issue introduced with the new feature.

Comment 1

[Paul Zühlcke [:pbz]]

Seems to be an issue with the window flags. The patch here fixes it for me: https://phabricator.services.mozilla.com/D76735

Comment 2

[Daniel Bodea [:danibodea]]

Yes, it is fixed in Nightly v78.0a1 from 2020-05-27, along with bug 1639997. Pushlog:

13:05.71 INFO: No more integration revisions, bisection finished.
13:05.72 INFO: First good revision: f4e08664070b8bc5f78fef734b8b064dcd53eb86
13:05.72 INFO: Last bad revision: d78ab1d85aedd026a6557d1dbf75deafc01ec843
13:05.72 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d78ab1d85aedd026a6557d1dbf75deafc01ec843&tochange=f4e08664070b8bc5f78fef734b8b064dcd53eb86

Comment 3

[Mike Conley (:mconley) (:⚙️)]

It's likely we'll need to re-open this once bug 1641546 lands.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

We'll probably want to hide this titlebar by default like we do with the Picture-in-Picture player window. I just need to figure out how to convince our GTK backend to do that.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1640973 - [Linux] Enable hidden system titlebar for dialogs r?mconley

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1640973 - Update WebRTC indicator properties for titlebar drawing on Linux GTK. r?pbz

Passing anything as an argument (even an empty array) to Services.ww.openWindow causes
the windowing system to assume that the opened window is a dialog. On Linux GTK, dialog
windows can't draw in their titlebars. Removing this empty array (which was never being
used in the first place), allows the window to be considered a "top-level" window on
Linux GTK.

On macOS and Windows, the window is still considered a dialog, since there's no problem
drawing in the titlebar on that platform.

We also add the drawintitlebar attribute to the HTML node of the indicator, which tells
the backend that we are definitely expecting to draw in the titlebar.

Depends on D78818

Comment 7

[Mike Conley (:mconley) (:⚙️)]

With bug 1641546 having landed, this is now regressed - but I have some patches in the works to address this case.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

stransky wrote here:

It's generally correct but I'd like to stress that we use a popup window config here and for pip window.

Hm, this might be problematic, and might explain why the window doesn't seem to minimize properly with this patch. There's a non-native minimize button in the indicator, and with this patch applied, the window does minimize, but doesn't seem to be restorable.

Ultimately, the effect we're trying to achieve here is a minimizable, always-on-top window without the native titlebar. Is that possible?

Comment 9

[Martin Stránský [:stransky] (ni? me)]

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

stransky wrote here:

It's generally correct but I'd like to stress that we use a popup window config here and for pip window.

Hm, this might be problematic, and might explain why the window doesn't seem to minimize properly with this patch. There's a non-native minimize button in the indicator, and with this patch applied, the window does minimize, but doesn't seem to be restorable.

Ultimately, the effect we're trying to achieve here is a minimizable, always-on-top window without the native titlebar. Is that possible?

Yes, that should be possible. I wonder why the Firefox PIP behaves differently here...may be caused by some window hint we pass there.
I'll look at it.

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Mike, I see the PIP window also can't be maximized/minimized by WM on X11. Do you want the same functionality for PIP?

Comment 11

[Martin Stránský [:stransky] (ni? me)]

The lack of minimize/maximize capability is caused by GDK_WINDOW_TYPE_HINT_UTILITY hint at least for PIP window.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

(In reply to Martin Stránský [:stransky] from comment #10)

Mike, I see the PIP window also can't be maximized/minimized by WM on X11. Do you want the same functionality for PIP?

Minimizing the PiP window is not something we offer with the buttons in the player, but I can't think of a reason why we'd want to prevent users from restoring the window if they've somehow managed to minimize it.

So presuming we still draw in the titlebar in the PiP player window, then I don't mind the player window having minimize / restore capabilities.

Comment 13

[Martin Stránský [:stransky] (ni? me)]

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

(In reply to Martin Stránský [:stransky] from comment #10)

Mike, I see the PIP window also can't be maximized/minimized by WM on X11. Do you want the same functionality for PIP?

Minimizing the PiP window is not something we offer with the buttons in the player, but I can't think of a reason why we'd want to prevent users from restoring the window if they've somehow managed to minimize it.

So presuming we still draw in the titlebar in the PiP player window, then I don't mind the player window having minimize / restore capabilities.

I have the patch ready, just do some testing before I post it. Let's keep the PIP as is and implement the indicator as dialog with hidden titlebar. That does not mix PIP with other window type.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

(In reply to Martin Stránský [:stransky] from comment #13)

I have the patch ready, just do some testing before I post it. Let's keep the PIP as is and implement the indicator as dialog with hidden titlebar. That does not mix PIP with other window type.

That's perfect, thank you!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56684b74f0ea
[Linux] Enable hidden system titlebar for dialogs r=jhorak

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/56684b74f0ea

Comment 17

[Camelia Badau [:cbadau], Desktop Test Engineering]

Reproduced the issue on Firefox Nightly 78.0a1 (2020-05-26).
I've tested on:

• latest Nightly 79.0a1 (2020-06-17): the issue is fixed
• Firefox 78 Beta 8 (buildID: 20200616235426): the issue is still reproducible here. Will this be considered to get an uplift to 78 or let it ride the train?

I've tested with privacy.webrtc.allowSilencingNotifications = true, privacy.webrtc.legacyGlobalIndicator = false and privacy.webrtc.sharedTabWarning = true.

Comment 18

[Daniel Bodea [:danibodea]]

I can confirm the same behavior as Camelia sais in the comment above.

Comment 19

[Mike Conley (:mconley) (:⚙️)]

We're going to let this ride the trains. Thanks!