Closed
Bug 1229727
Opened 8 years ago
Closed 8 years ago
[OS X] When going into video full screen while already being in OS X full screen mode, the title bar overlaps our messaging
Categories
(Firefox :: Shell Integration, defect, P4)
Tracking
()
VERIFIED
FIXED
Firefox 48
People
(Reporter: phlsa, Assigned: timdream)
References
(Blocks 1 open bug)
Details
(Whiteboard: [qx])
Attachments
(3 files, 1 obsolete file)
103.78 KB,
image/png
|
Details | |
3.19 MB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
dao
:
review+
timdream
:
ui-review+
|
Details |
STR: - Go to a video on youtube.com - Put the browser window into full screen mode (green circle at the top left of the window) - Play the video and put it into full screen using the controls on the youtube page - Move the mouse to the top edge of the screen Expected: The button to exit full screen mode appears. Actual: The button appears, but it is overlapped by the title bar that also slides down when moving the mouse to the top edge. (see attachment)
Updated•8 years ago
|
Priority: -- → P4
Assignee | ||
Comment 1•8 years ago
|
||
The bug comment here does not specify the desired result in clear -- my approach here simply makes the warning travel further down to 80px when we are in DOM fullscreen AND OSX Lion full screen, so it would not be overlapped by title bar. Tested manually when in and out of DOM full screen and move the cursor to the top of the screen. :phlsa, nice to meet you! I don't really know how ui-review works in Firefox land -- do I need to prepare a test build for you, or you would be able to do so in your office?
Assignee | ||
Comment 2•8 years ago
|
||
Position of the warning after patch is applied.
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8735411 [details] [diff] [review] bug1229727.patch Hey Tim, nice to meet you too! Thanks for attaching the screen shot! I think that's sufficient for the UI review of this work. From the CSS (and your comment) it looks like that this only applies when actually in Lion full screen mode, which is exactly how it should work :)
Attachment #8735411 -
Flags: ui-review?(philipp) → ui-review+
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8735411 [details] [diff] [review] bug1229727.patch Dave, would you be able to review this? You are one of the suggested reviewers. Thanks!
Attachment #8735411 -
Flags: review?(dtownsend)
Updated•8 years ago
|
Attachment #8735411 -
Flags: review?(dtownsend) → review?(dao)
Comment 5•8 years ago
|
||
Why is the title bar there in the first place? DOM fullscreen should always cover the entire screen, shouldn't it?
Blocks: fx-fullscreen
Comment 6•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #5) > Why is the title bar there in the first place? DOM fullscreen should always > cover the entire screen, shouldn't it? That's OS X's full screen mode. If you enter the full screen mode on OS X, we use its native full screen mode, for which the system creates a new Space for the application. In that Space, the menubar and the title bar are always displayed when you hover the mouse on the top of the screen. And if you enter DOM fullscreen when you are already in the full screen mode, we just make the content cover the whole window, so in that case, the browser window itself is still in the native full screen mode. All video players as well as Safari have that in full screen, so showing that is expected behavior on OS X.
Assignee | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/45555/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45555/
Attachment #8740085 -
Flags: review?(dao)
Assignee | ||
Updated•8 years ago
|
Attachment #8740085 -
Flags: ui-review+
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8735411 [details] [diff] [review] bug1229727.patch Moving the patch to MozReview.
Attachment #8735411 -
Attachment is obsolete: true
Attachment #8735411 -
Flags: review?(dao)
Updated•8 years ago
|
Attachment #8740085 -
Flags: review?(dao) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7ea05f1a9b98
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Updated•8 years ago
|
Flags: qe-verify+
Comment 11•8 years ago
|
||
Reproduced on FX 47 release. Verified fixed FX 48b2, OS X 10.10.5.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•