Closed Bug 1826694 Opened 1 year ago Closed 1 year ago

PIP window not support rounded corners in last few Nightly with MicaForEveryone installed, still works in Stable.

Categories

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

Firefox 113
defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr102 --- unaffected
firefox112 --- unaffected
firefox113 --- affected
firefox114 --- affected

People

(Reporter: skibinvitaliy, Unassigned)

References

(Regression)

Details

(Keywords: regression)

Steps to reproduce:

Open any video, switch to the Picture_in_Picture window.

Actual results:

The Picture_in_Picture window corners are straight, not rounded.
The PIP player window no longer follows "system rules" and does not support rounded corners in the last few Nightly builds, although it still works great in a Stable builds..
Which makes me believe that this is related to the latest PIP player improvements and new features.
https://i.imgur.com/Vkh9DhU.png

Expected results:

The Picture_in_Picture window corners are rounded, not straight.
Now, this is not a bug technically, but could you pay attention to this and fix this behavior?
PS I just want to note that having rounded corners in the PIP player window was possible using a third-party app "Mica For Everyone".

https://i.imgur.com/Y5PDITh.png

Group: firefox-core-security

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 skibinvitaliy, thanks for filing a ticket. Are you able to run mozregression and share the regression range that you get?
We have a tutorial on how to get started: https://mozilla.github.io/mozregression/quickstart.html.

This would help us determine potential culprits for the defect.

Severity: -- → S3
Flags: needinfo?(skibinvitaliy)

(In reply to kpatenio from comment #2)

Hi skibinvitaliy, thanks for filing a ticket. Are you able to run mozregression and share the regression range that you get?
We have a tutorial on how to get started: https://mozilla.github.io/mozregression/quickstart.html.

This would help us determine potential culprits for the defect.

Hi.
Thank you for reacting.
I did what you asked, but I could not figure out how to get and share a regression range.
So, here is a link to a full log instead, hope this helps.
https://pastebin.com/raw/2nhaydpv

Flags: needinfo?(skibinvitaliy)

Thanks! The bottom of the log links to Bug 1823350.

@emilio - do you have time to look into this?

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(emilio)
Keywords: regression
Regressed by: 1823350

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

What DE are you on? Can you attach about:support please? Thanks

Flags: needinfo?(emilio) → needinfo?(skibinvitaliy)

(keeping ni? to take a look tomorrow)

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

What DE are you on? Can you attach about:support please? Thanks

Yes, here it is:
https://pastebin.com/1vqfyde4

Flags: needinfo?(skibinvitaliy)

Ah, ok, so you're in win10 and using https://github.com/MicaForEveryone/MicaForEveryone, which is what seemed to be adding the rounded corners.

The thing that changed the behavior in that patch is changing titlebar=yes to titlebar=no on Windows (which effectively is a change in the window style, but which reflects reality).

I think MicaForEveryone is not dealing with that (which might make sense because the PiP window has no titlebar). But I think this is not really something we want to workaround on our end either. MicaForEveryone doesn't work with other non-native titlebars afaict, and PiP never had rounded corners on a default setup, so I think this is WONTFIX.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(emilio)
Resolution: --- → WONTFIX
Summary: PIP window not support rounded corners in last few Nightly, still works in Stable. → PIP window not support rounded corners in last few Nightly with MicaForEveryone installed, still works in Stable.

Eh, it's sad then.
So, you do not have a built-in support for rounded corners and do not plan to implement it.
This could be done using a third-party application, but you made changes and broke it.
And you do not plan to fix it.
Did I understand you correctly?
PS Please do not perceive my words as rude, I did not mean anything like that, I just want to understand this more precisely..

Just wondering why you need to make this change if the PIP window did not have and still has no visible titlebar anyway?
PPS No, I'm on Windows 11, but nevertheless.
Thank you for the clarification.

So, you do not have a built-in support for rounded corners and do not plan to implement it.

We do have support to make the PiP window rounded if someone / UX / whoever wanted, afaict (via regular CSS), the same way we implement rounded corners in Linux. I don't know about plans to implement it on windows 11 tho.

This could be done using a third-party application, but you made changes and broke it.

That application is basically a windows titlebar hack. We annotated the PiP window as having no titlebar, and it broke that particular application (which already doesn't work on regular Firefox windows unless you enable the native titlebar, if I'm not misreading this).

And you do not plan to fix it.

I don't think it's worth reverting, correct, because the PiP window has no titlebar.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

We do have support to make the PiP window rounded if someone / UX / whoever wanted, afaict (via regular CSS), the same way we implement rounded corners in Linux. I don't know about plans to implement it on windows 11 tho.

Yeah, it doesn't work on Windows 10/11, since there is still a container (background) remains behind with square corners that cannot be modified "via regular CSS".. Or at least no one knows or does not say how it can be done..
Here is a small discussion on this subject:
https://www.reddit.com/r/FirefoxCSS/comments/xo82s8/how_to_stylize_the_pip_player_window_again/

That application is basically a windows titlebar hack. We annotated the PiP window as having no titlebar, and it broke that particular application (which already doesn't work on regular Firefox windows unless you enable the native titlebar, if I'm not misreading this).

I wouldn't use a third-party stuff with great joy, but I have to do this since nothing is perfect in this world.
And there is always someone who is sure that some of the things are unnecessary to the end user even if the user asks for it.
Right..

I don't think it's worth reverting, correct, because the PiP window has no titlebar.

This part remains not clear to me.
The PIP window did not have and still does not have a visible titlebar, so what exactly it changes?
Any way to revert it not globally, but at least locally?
Via about:config, CSS, JS?

(In reply to skibinvitaliy from comment #12)

Yeah, it doesn't work on Windows 10/11, since there is still a container (background) remains behind with square corners that cannot be modified "via regular CSS".. Or at least no one knows or does not say how it can be done..

Ah, that's a silly Windows-XP-era restriction, and you're right it can't be worked around. I sent a patch removing that restriction in bug 1829006.

With those and this code I can get rounded corners on PiP windows:

diff --git a/toolkit/themes/shared/pictureinpicture/player.css b/toolkit/themes/shared/pictureinpicture/player.css
index a2687f3771af6..018420f615770 100644
--- a/toolkit/themes/shared/pictureinpicture/player.css
+++ b/toolkit/themes/shared/pictureinpicture/player.css
@@ -3,7 +3,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

 :root {
-  --player-bg-color: #000;
   --player-control-icon-fill: #fff;
   --player-control-item-half-width: clamp(calc(16px / 2), calc(10vmax / 2), calc(32px / 2));
   --player-control-item-height: clamp(16px, 10vmax, 32px);
@@ -22,6 +21,8 @@ button::-moz-focus-inner {
 }

 body {
+  border-radius: 10px;
+  overflow: clip;
   margin: 0;
 }

(In reply to Emilio Cobos Álvarez (:emilio) from comment #13)

Ah, that's a silly Windows-XP-era restriction, and you're right it can't be worked around. I sent a patch removing that restriction in bug 1829006.

Oh, that's great, thanks.
Let's see if this be eventually Implemented..

With those and this code I can get rounded corners on PiP windows:

Do you mean this already works?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #13)

Ah, that's a silly Windows-XP-era restriction, and you're right it can't be worked around. I sent a patch removing that restriction in bug 1829006.

Hi.
Although, as I can see (from bug 1829006), some changes have been made, it still does not work.
I mean, this background cannot be modified, the corners cannot be rounded or it cannot be hidden via transparency.
What can I do?

That bug is still not closed because https://phabricator.services.mozilla.com/D175952 is waiting on review.

See Also: → 1829006

(In reply to Emilio Cobos Álvarez (:emilio) from comment #16)

That bug is still not closed because https://phabricator.services.mozilla.com/D175952 is waiting on review.

Ah, ok, we'll wait then, thanks..

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