966698 - [Australis] If MenuBar displayed, Mouse pointer does not react at the top pixel In Fullscreen mode(F11)

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Alice0775 White]

This problem happens on Windows7 classic and Aero visual style.

Steps To Reproduce:
1. Enable Menu Bar
2. Enter Fullscreen (press key F11)
3. Move mouse pointer to top of screen
4. Wait for a while so that toolbar appears
5. Right click (do not move mouse pointer)
    --- nothing happens....
 
6. Move mouse pointer to top of screen
7. Wait for a while so that toolbar appears
8. Move mouse pointer to near window controls
    --- no hover effect...

Actual Results:
No fullscreen context menu
No hover effect of window controls

Expected Results:
Fullscreen context menu should pop up
Window controls should be styled hover style. and be reacted by click


The problem exists since UX marge day

Comment 1

[Alice0775 White]

Attachment: screenshot

Probably , extra gap exist at the top of screen in fullscreen mode if MenuBar enabled.

Comment 2

[Alice0775 White]

The problem is margin-top is not considered in fullscreen mode.

http://hg.mozilla.org/mozilla-central/annotate/3e40f7389d1b/browser/themes/windows/browser.css#l36

36 #toolbar-menubar:not([autohide="true"]) ~ #TabsToolbar,
37 #toolbar-menubar[autohide="true"]:not([inactive]) ~ #TabsToolbar {
38  margin-top: 3px;
39 }

Comment 3

[Alice0775 White]

margin-top should be 0px in fullscreen mode

Comment 4

[Benjamin Kerensa [:bkerensa]]

Gijs, Can you have a look at this?

Comment 5

[:Gijs (he/him)]

No, because as far as my sources can tell me, the context menu isn't fully unavailable, and we have worse (and therefore higher priority) bugs to fix right now. I'll be happy to review a patch.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Alice0775 White from comment #2)
> The problem is margin-top is not considered in fullscreen mode.
> 
> http://hg.mozilla.org/mozilla-central/annotate/3e40f7389d1b/browser/themes/
> windows/browser.css#l36
> 
> 36 #toolbar-menubar:not([autohide="true"]) ~ #TabsToolbar,
> 37 #toolbar-menubar[autohide="true"]:not([inactive]) ~ #TabsToolbar {
> 38  margin-top: 3px;
> 39 }

Seem like we could simply add :not([moz-collapsed=true]) to these selectors.

Comment 7

[:Gijs (he/him)]

If we're tracking this, it should probably show up on the tracker...

Comment 8

[Dão Gottwald [:dao]]

Attachment: patch

Comment 9

[:Gijs (he/him)]

Comment on attachment 8376696 [details] [diff] [review]
patch

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

::: browser/themes/windows/browser.css
@@ +33,5 @@
>  /* We want a 4px gap between the TabsToolbar and the toolbar-menubar when the
>     toolbar-menu is displayed, and a 16px gap when it is not. 1px is taken care
>     of by the (light) outer shadow of the tab, the remaining 3/15 are these margins. */
> +#toolbar-menubar:not([moz-collapsed=true]):not([autohide=true]) ~ #TabsToolbar,
> +#toolbar-menubar:not([moz-collapsed=true])[autohide=true]:not([inactive]) ~ #TabsToolbar {

Nit: seems to me like we quote attribute values everywhere, and we should do that here, too.

Comment 10

[:Gijs (he/him)]

(also, I find the relying on that attribute a little odd - this is the only place (bar the implementation of the actual functionality of that attribute in xul.css) where we use it - but I suppose making the rule rely on attributes on the window or toolbox would be strictly worse from a selector perf perspective. Don't know of an alternative solution and adding extra attributes to this still seems silly too)

Comment 11

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #9)
> Comment on attachment 8376696 [details] [diff] [review]
> patch
> 
> Review of attachment 8376696 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/windows/browser.css
> @@ +33,5 @@
> >  /* We want a 4px gap between the TabsToolbar and the toolbar-menubar when the
> >     toolbar-menu is displayed, and a 16px gap when it is not. 1px is taken care
> >     of by the (light) outer shadow of the tab, the remaining 3/15 are these margins. */
> > +#toolbar-menubar:not([moz-collapsed=true]):not([autohide=true]) ~ #TabsToolbar,
> > +#toolbar-menubar:not([moz-collapsed=true])[autohide=true]:not([inactive]) ~ #TabsToolbar {
> 
> Nit: seems to me like we quote attribute values everywhere, and we should do
> that here, too.

not everywhere:

http://mxr.mozilla.org/mozilla-central/search?string=%3Dtrue]&find=theme

Nor do I think we should do it everywhere.

Comment 12

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/70e5e705befd

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/70e5e705befd

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8376696 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879162
User impact if declined: see comment 0
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 15

[:Gijs (he/him)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8f0d1056bc1e

Comment 16

[Ioana (away)]

Reproduced on the February 2nd Nightly. Verified as fixed on the latest Beta, Aurora and Nightly, using both the classic and Aero themes of Windows 7 x86_64.