Closed Bug 1177185 Opened 9 years ago Closed 9 years ago

Hidden titlebar appears when exiting DOM Fullscreen

Categories

(Firefox :: General, defect)

41 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 + verified

People

(Reporter: over68, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(2 files)

Steps to reproduce:

1. Go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/kwtoe6r53d.mp4.
2. Switch to fullscreen mode, then exit the full screen.

Note the space above the tab.

Screenshot https://dl.dropboxusercontent.com/u/95157096/85f61cf7/c0yy7zbv31.png
(In reply to blinky from comment #1)
> Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=3fca18104696&tochange=4b47c3f074a3

You should bisect m-c first and then bisect m-i or fx-team.
Flags: needinfo?(alice0775)
[Tracking Requested - why for this release]:

Thanks.
So, This was regressed by Bug 1176233
Blocks: 1176233
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Product: Core → Firefox
Attached file FullcreenAPI.html
Summary: Fullscreen mode for video leads to move tab bar to the bottom → Hidden titlebar appears when exiting DOM Fullscreen
Flags: needinfo?(dao)
Attached patch patchSplinter Review
The problem is that we're removing the inDOMFullscreen attribute too late. I'm not sure if there's a good way to address this. This patch is basically just a workaround.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Flags: needinfo?(dao)
Attachment #8626210 - Flags: review?(quanxunzhen)
Comment on attachment 8626210 [details] [diff] [review]
patch

Review of attachment 8626210 [details] [diff] [review]:
-----------------------------------------------------------------

We probaby could change the order to dispatch events again, but the order really has a deeper effect than I thought. I don't dare to change that anymore at this moment.

The UI shouldn't be in DOM fullscreen state if it is not in fullscreen state, hence this workaround looks reasonable anyway.
Attachment #8626210 - Flags: review?(quanxunzhen) → review+
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #7)
> We probaby could change the order to dispatch events again, but the order
> really has a deeper effect than I thought. I don't dare to change that
> anymore at this moment.

Sounds like we should still have a bug on file for that even if we wouldn't uplift that change?
(In reply to Dão Gottwald [:dao] from comment #8)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #7)
> > We probaby could change the order to dispatch events again, but the order
> > really has a deeper effect than I thought. I don't dare to change that
> > anymore at this moment.
> 
> Sounds like we should still have a bug on file for that even if we wouldn't
> uplift that change?

No, not necessary to be an individual bug. I have no clear idea about what's the perfect order. Probably it doesn't even exist.

We may change that once it proves that there is some other order obviously better than the current one which requires fewer workarounds on frontend UI side.
https://hg.mozilla.org/mozilla-central/rev/521e4737222b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment on attachment 8626210 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1176233
[User impact if declined]: see comment 0
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: trivial CSS patch, not risky
[String/UUID change made/needed]: none
Attachment #8626210 - Flags: approval-mozilla-beta?
This bug depends on bug 1176233. Waiting for test results there before uplifting either patch.
Adding a tracking FF41 as this is a regression.
Also requesting QE to verify the fix and related scenarios to ensure there is no regression.
Flags: qe-verify+
Verified as fixed using Aurora 41.0a2 under Win 7 64-bit.
Status: RESOLVED → VERIFIED
Comment on attachment 8626210 [details] [diff] [review]
patch

As the fix is now verified, let's get this into beta3. Beta+
Attachment #8626210 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I was able to reproduce this issue on Firefox 41.0a1 (2015-06-24) under Windows 7 64-bit.
Verified fixed on Firefox 40 Beta 4 (20150713153304) using Windows 7 64-bit.
You need to log in before you can comment on or make changes to this bug.