610769 - Align the Firefox menu with the Firefox button

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Planned alignment at 8x

Slight alignment change for the Firefox menu: goal is to have it left aligned with the dark edge of the button, and flush with the bottom.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: v1

Not sure if these are the desired margins or not. It just moves the menu 1px to the right and 1px up.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: screenshot with patch

And what it looks like, zoomed in 800%. (Applied via Stylish against beta7, if that matters. Didn't feel like building from trunk with the change, and 7-zip still won't let me in to omni.jar to make the change manually...)

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: screenshot v2

Oops, apparently I just zoomed in 800%, and didn't enlarge the picture itself...

Let's try this again?

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 489674 [details] [diff] [review]
v1

Use margin-top and -moz-margin-start instead of the margin shorthand.

Also test this with Classic -- the margin should only be needed with Aero.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: v2 to avoid the shorthand

Changes CSS to remove shorthand

> Also test this with Classic -- the margin should only be needed with Aero.

You sure? Looks like there was a 1px gap between the menu and the button (and a 1px offset from the left edge) on the Win7 Classic theme without this patch applying to it as well. I don't have a copy of XP handy to see how it looks.


Do any of the other OS themes have the app button accessible?

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> You sure? Looks like there was a 1px gap between the menu and the button (and a
> 1px offset from the left edge) on the Win7 Classic theme without this patch
> applying to it as well.

That's not what I see and not what browser-aero.css says. It adds the white outer border (which your compensating here) only for -moz-windows-default-theme.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: v3

Now in browser-aero.css.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 489893 [details] [diff] [review]
v3

Yes, this should do it, thanks.

>+  #appmenu-popup {
>+    margin-top: -1px;
>+   -moz-margin-start: 1px;
>+  }

nit: add another space before -moz-margin-start

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: v3 without nit

Adds the space.

Carrying forward r+

Comment 10

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/e6809237f52c