Closed
Bug 1177185
Opened 9 years ago
Closed 9 years ago
Hidden titlebar appears when exiting DOM Fullscreen
Categories
(Firefox :: General, defect)
Tracking
()
VERIFIED
FIXED
Firefox 41
People
(Reporter: over68, Assigned: dao)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
xidorn
:
review+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Flags: needinfo?(alice0775)
Comment 2•9 years ago
|
||
(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)
Regression range (mozilla-central):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a38c23ccca0a&tochange=be81b8d6fae9
Regression range (fx-team):
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=391241169bec&tochange=1f7db5d87e80
Comment 4•9 years ago
|
||
[Tracking Requested - why for this release]:
Thanks.
So, This was regressed by Bug 1176233
Blocks: 1176233
Status: UNCONFIRMED → NEW
status-firefox41:
--- → affected
tracking-firefox41:
--- → ?
Ever confirmed: true
Keywords: regression
Product: Core → Firefox
Comment 5•9 years ago
|
||
Updated•9 years ago
|
Summary: Fullscreen mode for video leads to move tab bar to the bottom → Hidden titlebar appears when exiting DOM Fullscreen
Updated•9 years ago
|
Flags: needinfo?(dao)
Assignee | ||
Comment 6•9 years ago
|
||
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 7•9 years ago
|
||
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+
Assignee | ||
Comment 8•9 years ago
|
||
(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?
Comment 10•9 years ago
|
||
(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.
Comment 11•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Assignee | ||
Comment 12•9 years ago
|
||
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?
Comment 13•9 years ago
|
||
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+
Comment 16•9 years ago
|
||
Verified as fixed using Aurora 41.0a2 under Win 7 64-bit.
Status: RESOLVED → VERIFIED
Comment 17•9 years ago
|
||
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+
Updated•9 years ago
|
status-firefox40:
--- → affected
Comment 18•9 years ago
|
||
Comment 19•9 years ago
|
||
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.
Description
•