1076626 - Global notification bars should be hidden in fullscreen mode

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[(no longer active)]

This causes, for example, for that infobar to stay at the bottom of the screen when a youtube video goes fullscreen.

Comment 1

[Dão Gottwald [:dao]]

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.

Comment 2

[Anirudh GP(:anirudhgp)]

Hi. I would like to patch this bug. This is my first so how do i go about it?

Comment 3

[Dão Gottwald [:dao]]

(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?

Comment 4

[Anirudh GP(:anirudhgp)]

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??

Comment 5

[Dão Gottwald [:dao]]

(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.

Comment 6

[Anirudh GP(:anirudhgp)]

Ok i'll work on it and get back.

Comment 7

[Anirudh GP(:anirudhgp)]

hey i need some help here. Where exactly do i specify the visibility as collapse in the browser.css file?

Comment 8

[Dão Gottwald [:dao]]

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

Comment 9

[Anirudh GP(:anirudhgp)]

ok so for example i can add in line 281: #main-window[inFullscreen] #global-notificationbox { visibility: collapse; ?

Comment 10

[Dão Gottwald [:dao]]

(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 '{'.

Comment 11

[Anirudh GP(:anirudhgp)]

ok yeah got it.

Comment 12

[Anirudh GP(:anirudhgp)]

Attachment: bug-1076626-fix.patch

Comment 13

[Dão Gottwald [: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;
}

Comment 14

[Anirudh GP(:anirudhgp)]

(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!

Comment 15

[Anirudh GP(:anirudhgp)]

Attachment: bug-1076626-fix.patch

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 8499507 [details] [diff] [review]
bug-1076626-fix.patch

Thanks!

Comment 17

[Anirudh GP(:anirudhgp)]

(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?

Comment 18

[Dão Gottwald [:dao]]

(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

Comment 19

[Anirudh GP(:anirudhgp)]

(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.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ab146717125b

Comment 21

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Nightly 35.0a1 (buildID: 20141005030205).

Comment 22

[Dave Townsend [:mossop]]

Looks like this patch added DOS line endings to browser.css :(

Comment 23

[Anirudh GP(:anirudhgp)]

(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?