Closed Bug 1076626 Opened 10 years ago Closed 10 years ago

Global notification bars should be hidden in fullscreen mode

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 35
Iteration:
35.3
Tracking Status
firefox35 --- verified

People

(Reporter: ehsan.akhgari, Assigned: anirudhgp, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][lang=css])

Attachments

(1 file, 1 obsolete file)

This causes, for example, for that infobar to stay at the bottom of the screen when a youtube video goes fullscreen.
Mentor: dao
Flags: firefox-backlog+
OS: Mac OS X → All
Hardware: x86 → All
Summary: The "Sync encountered an error" footer infobar stays there when the window goes fullscreen → Global notification bars should be hidden in fullscreen mode
Whiteboard: [good first bug][lang=js]
This should be fixed by setting "visibility: collapse;" for "#main-window[inFullscreen] #global-notificationbox" and "#main-window[inFullscreen] #high-priority-global-notificationbox" in browser/base/content/browser.css.
Component: Toolbars and Customization → General
Flags: qe-verify+
Whiteboard: [good first bug][lang=js] → [good first bug][lang=css]
Hi. I would like to patch this bug. This is my first so how do i go about it?
(In reply to anirudh.gp from comment #2)
> Hi. I would like to patch this bug. This is my first so how do i go about it?

Have you read <https://developer.mozilla.org/en-US/docs/Introduction>?
Do you know CSS?
Does comment 1 make sense to you?
Yeah I have read that and set up everything on my pc. I know css. What I understood from the comment is that you have to set the visibility to collapse in the places mentioned. Is this correct??
(In reply to anirudh.gp from comment #4)
> Yeah I have read that and set up everything on my pc. I know css. What I
> understood from the comment is that you have to set the visibility to
> collapse in the places mentioned. Is this correct??

Yes, correct.
Ok i'll work on it and get back.
hey i need some help here. Where exactly do i specify the visibility as collapse in the browser.css file?
You need to add a new rule with the selectors I mentioned in comment 1. It doesn't matter much where you add that rule. I'd probably do it here: http://hg.mozilla.org/mozilla-central/annotate/b85c260821ab/browser/base/content/browser.css#l254
ok so for example i can add in line 281: #main-window[inFullscreen] #global-notificationbox { visibility: collapse; ?
(In reply to anirudh.gp from comment #9)
> ok so for example i can add in line 281: #main-window[inFullscreen]
> #global-notificationbox { visibility: collapse; ?

Basically yes, although there should be a line break after '{'.
ok yeah got it.
Attached patch bug-1076626-fix.patch (obsolete) — Splinter Review
Attachment #8499465 - Flags: review?(dao)
Comment on attachment 8499465 [details] [diff] [review]
bug-1076626-fix.patch

>+#main-window[inFullscreen] #global-notificationbox {
>+    visibility:collapse;
>+}
>+
>+#main-window[inFullscreen] #high-priority-global-notificationbox {
>+    visibility:collapse;
>+}

Great, just a few small things:

- can you move this to line 254? This is where I linked to in comment 8
- indent with two spaces instead of four
- add a space after ':'
- merge the two rules into one, e.g.:

a,
b {
  foo: bar;
}

instead of:

a {
  foo: bar;
}

b {
  foo: bar;
}
Attachment #8499465 - Flags: review?(dao) → review+
(In reply to Dão Gottwald [:dao] from comment #13)
> Comment on attachment 8499465 [details] [diff] [review]
> bug-1076626-fix.patch
> 
> >+#main-window[inFullscreen] #global-notificationbox {
> >+    visibility:collapse;
> >+}
> >+
> >+#main-window[inFullscreen] #high-priority-global-notificationbox {
> >+    visibility:collapse;
> >+}
> 
> Great, just a few small things:
> 
> - can you move this to line 254? This is where I linked to in comment 8
> - indent with two spaces instead of four
> - add a space after ':'
> - merge the two rules into one, e.g.:
> 
> a,
> b {
>   foo: bar;
> }
> 
> instead of:
> 
> a {
>   foo: bar;
> }
> 
> b {
>   foo: bar;
> }

Ok i'll do that right away!
Attachment #8499507 - Flags: review?(dao)
Comment on attachment 8499507 [details] [diff] [review]
bug-1076626-fix.patch

Thanks!
Attachment #8499507 - Flags: review?(dao) → review+
Attachment #8499465 - Attachment is obsolete: true
(In reply to Dão Gottwald [:dao] from comment #16)
> Comment on attachment 8499507 [details] [diff] [review]
> bug-1076626-fix.patch
> 
> Thanks!

Is there anything else for me to do with regard to this bug or is it done?
(In reply to anirudh.gp from comment #17)
> (In reply to Dão Gottwald [:dao] from comment #16)
> > Comment on attachment 8499507 [details] [diff] [review]
> > bug-1076626-fix.patch
> > 
> > Thanks!
> 
> Is there anything else for me to do with regard to this bug or is it done?

Normally you'd add the checkin-needed keyword (at the top of this page) and wait for someone to come along to land your patch. In this case, I'll just do it right away:

https://hg.mozilla.org/integration/fx-team/rev/ab146717125b
Assignee: nobody → anirudh.gp
(In reply to Dão Gottwald [:dao] from comment #18)
> (In reply to anirudh.gp from comment #17)
> > (In reply to Dão Gottwald [:dao] from comment #16)
> > > Comment on attachment 8499507 [details] [diff] [review]
> > > bug-1076626-fix.patch
> > > 
> > > Thanks!
> > 
> > Is there anything else for me to do with regard to this bug or is it done?
> 
> Normally you'd add the checkin-needed keyword (at the top of this page) and
> wait for someone to come along to land your patch. In this case, I'll just
> do it right away:
> 
> https://hg.mozilla.org/integration/fx-team/rev/ab146717125b

Thanks a lot.
https://hg.mozilla.org/mozilla-central/rev/ab146717125b
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Iteration: --- → 35.3
Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Nightly 35.0a1 (buildID: 20141005030205).
Status: RESOLVED → VERIFIED
QA Contact: camelia.badau
Looks like this patch added DOS line endings to browser.css :(
(In reply to Dave Townsend [:mossop] from comment #22)
> Looks like this patch added DOS line endings to browser.css :(

How do I fix it?
Blocks: 1760030
You need to log in before you can comment on or make changes to this bug.