1

PIP button z-index not respected with semi-transparent div

 2 years ago
source link: https://bugzilla.mozilla.org/show_bug.cgi?id=1742585
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Closed Bug 1742585 Opened 2 months ago Closed 15 days ago

PIP button z-index not respected with semi-transparent div

Categories

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

Toolkit ▾
Picture-in-Picture ▾

Firefox 95

Tracking

(bug RESOLVED as FIXED)

RESOLVED FIXED

97 Branch

Tracking Status firefox97 --- fixed

People

(Reporter: me, Assigned: rhopkinsdev)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidefe-MR1-2022])

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

Create a video element, and add a semi-transparent div on the right.
Click on the PiP button that is below the div.

Actual results:

The PiP window is opened.

Expected results:

The PiP window should not be opened since there is a div with an higher z-index on top of the PiP button.

default.jpg

Reporter

Comment 2

2 months ago

It causes issues with custom settings dialog displayed on top of video elements (for example PeerTube: https://framatube.org/w/kkGMgK9ZtnKfYAgnEtQxbv). Weirdly, youtube doesn't seem affected. I don't know how they manage to not trigger the PiP button when the settings dialog is opened.

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core

Mike, do we even support that?
Thanks.

Flags: needinfo?(mconley)

IIRC this is by design.

Component: Widget: Gtk → Picture-in-Picture
Product: Core → Toolkit

Possibly it should be smaller. We have to balance carefully - some sites use semi-transparent overlays to represent the "paused" state of their videos, which we still want users to be able to use PiP through.

I did a quick test using a default visibility threshold of 0.8. This allows the testcase to not cause the toggle to work, while still keeping the toggle working on YouTube, Vimeo, DailyMotion, Apple Trailers, Twitch and a few news sites that I did a cursory test on.

If we did decide to decrease the threshold a little bit, we'd probably want this to be a pref in case we need to quickly rollout a fix.

What do you think, rtestard?

Test builds with 0.8 toggle visibility threshold:

Linux 64: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/f_dpjJDBSout6uHy3OQbyA/runs/0/artifacts/public/build/target.tar.bz2
macOS: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/ILynuEEBQvqkBV8HwKOhiQ/runs/0/artifacts/public/build/target.dmg
Win 64: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/KJ2nSK15T3-vWS0JsU1E-w/runs/1/artifacts/public/build/target.zip

Flags: needinfo?(mconley) → needinfo?(rtestard)

Hi Ania, this might be of some interest to you, too.

Flags: needinfo?(amininkova)

I am hesitant to introduce a pref for threshold transparency, as I imagine that users' knee-jerk reaction to the non-clickable PiP in the paused state button won't be going to Settings.

A cursory look into SUMO and Reddit doesn't show that users struggle with the current situation - from what I understand, at its worst, it creates a need for the secondary click on a setting that directly overlaps with the PiP button.

We can roll out an experiment with a default visibility threshold of 0.8 (thank you for the build, Mike, the threshold looks good) and see if it affects PiP usage. Still, since we don't have granular analytics, we'll have to rely on user reports to understand what video websites it breaks.

Mike, how did we initially decide on the default threshold? I wonder if rolling out the 0.9 override for youtube for everyone could be a compromise?

Flags: needinfo?(amininkova)

I am hesitant to introduce a pref for threshold transparency

To be clear, when I said "pref", I didn't mean to expose it in about:preferences. It would be something in about:config, but the primary advantage is that we could roll out changes to it via RemoteSettings.

Mike, how did we initially decide on the default threshold? I wonder if rolling out the 0.9 override for youtube for everyone could be a compromise?

The default threshold of 1.0 was chosen, as it granted the user the most power with the toggle (with the trade-off being sites like YouTube needing a special threshold for their semi-transparent menus).

So how about we:

  1. Default to a 0.9 threshold
  2. Store that value in a way that we could easily rollback using RemoteSettings

and then deploy. If we're happy with everyone being at 0.9, then we can remove the YouTube-specific threshold rule.

That compromise works for me. Does it work for you?

Flags: needinfo?(rtestard) → needinfo?(amininkova)

My bad, being able to control the value via RemoteSettings would be great.

Yes, that works great for me.

Flags: needinfo?(amininkova)
Whiteboard: [fidefe-MR1-2022]

Hey Ania,

Did you have a sense of what priority this should be at?

Severity: -- → S4
Flags: needinfo?(amininkova)
Flags: needinfo?(amininkova)
Priority: -- → P3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: P3 → --
Priority: -- → P3
Assignee: nobody → rhopkinsdev
Status: NEW → ASSIGNED
Attachment #9256651 - Attachment description: Bug 1742585 - Add PiP toggle visibility threshold preference r?mhowell → Bug 1742585 - Add PiP toggle visibility threshold preference r?kpatenio, mtigley
Attachment #9256651 - Attachment description: Bug 1742585 - Add PiP toggle visibility threshold preference r?kpatenio, mtigley → Bug 1742585 - Add PiP toggle visibility threshold preference r?kpatenio,mtigley
Status: ASSIGNED → RESOLVED
Closed: 15 days ago
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch
You need to log in before you can comment on or make changes to this bug.

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK