Closed Bug 966698 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox :: Theme, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox28 --- unaffected
firefox29 + verified
firefox30 --- verified

People

(Reporter: alice0775, Assigned: dao)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [Australis:P3])

Attachments

(2 files)

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
Summary: [Austraris] If MenuBar displayed, Mouse pointer does not react at the top pixel In Fullscreen mode(F11) → [Australis] If MenuBar displayed, Mouse pointer does not react at the top pixel In Fullscreen mode(F11)
Attached image screenshot
Probably , extra gap exist at the top of screen in fullscreen mode if MenuBar enabled.
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 }
Blocks: 879162
margin-top should be 0px in fullscreen mode
Gijs, Can you have a look at this?
Flags: needinfo?(gijskruitbosch+bugs)
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.
Flags: needinfo?(gijskruitbosch+bugs)
Whiteboard: [Australis:P4]
(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.
Component: Toolbars and Customization → Theme
Whiteboard: [Australis:P4] → [Australis:P4][good first bug][lang=css][mentor=dao]
Version: 29 Branch → Trunk
If we're tracking this, it should probably show up on the tracker...
Whiteboard: [Australis:P4][good first bug][lang=css][mentor=dao] → [Australis:P3][good first bug][lang=css][mentor=dao]
Assignee: nobody → dao
Whiteboard: [Australis:P3][good first bug][lang=css][mentor=dao] → [Australis:P3]
Attached patch patchSplinter Review
Attachment #8376696 - Flags: review?(gijskruitbosch+bugs)
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.
Attachment #8376696 - Flags: review?(gijskruitbosch+bugs) → review+
(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)
(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.
https://hg.mozilla.org/mozilla-central/rev/70e5e705befd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
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
Attachment #8376696 - Flags: approval-mozilla-aurora?
Attachment #8376696 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Contact: cornel.ionce
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.
Status: RESOLVED → VERIFIED
QA Contact: cornel.ionce → ioana.budnar
You need to log in before you can comment on or make changes to this bug.