Since the open state was introduced by bug 985509, the styling used on Win 7 is not the right one. It seems that the Win 8 one is used instead.

This is most likely happening because the same rules need to be duplicated for Windows 7, and until then the Windows 8 ones will be the only ones applying and thus aren't being overridden.

Hey Tim, just to let you know that we'll need a fix for this bug in order to uplift bug 985509 to Aurora. If you could please put this bug at the top of your list, it would be greatly appreciated! :)

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2) > Hey Tim, just to let you know that we'll need a fix for this bug in order to > uplift bug 985509 to Aurora. If you could please put this bug at the top of > your list, it would be greatly appreciated! :) Well I was thinking of addressing this in bug 949151 so I'll have less re-basing work to do.

Feel free to steal this in case I don't create a patch in time.

I had a much busier schedule than I expected. Sorry for the delay :/

Hey Jared, it seems that you are busy :) Should I defer this review to someone else ?

Comment on attachment 8420308 [details] [diff] [review] Patch Review of attachment 8420308 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay on review here, I was away from my Win7 machine for quite some time. ::: browser/themes/windows/browser.css @@ +768,5 @@ > border-top-left-radius: 0; > border-bottom-left-radius: 0; > } > > + #nav-bar .toolbarbutton-1:not([disabled=true]) > .toolbarbutton-menubutton-button[open] + .toolbarbutton-menubutton-dropmarker > .dropmarker-icon, As this is a less-than-Windows8 issue, why does this line need to be added since it will also apply to Windows8 here?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7) > Comment on attachment 8420308 [details] [diff] [review] > Patch > > Review of attachment 8420308 [details] [diff] [review]: > ----------------------------------------------------------------- > > Sorry for the delay on review here, I was away from my Win7 machine for > quite some time. > > ::: browser/themes/windows/browser.css > @@ +768,5 @@ > > border-top-left-radius: 0; > > border-bottom-left-radius: 0; > > } > > > > + #nav-bar .toolbarbutton-1:not([disabled=true]) > .toolbarbutton-menubutton-button[open] + .toolbarbutton-menubutton-dropmarker > .dropmarker-icon, > > As this is a less-than-Windows8 issue, why does this line need to be added > since it will also apply to Windows8 here? In case you didn't notice, there's 2 space indenting everywhere. Which means it's included in the media query. I think it's the patch formatting that confuses you.

Comment on attachment 8420308 [details] [diff] [review] Patch Review of attachment 8420308 [details] [diff] [review]: ----------------------------------------------------------------- I had actually applied the patch to get more context but ended up looking at the wrong ruleset. Thanks for reaffirming. I tested it and it looks good.

hey guys, do we need a try link/run for this change ?

(In reply to Carsten Book [:Tomcat] from comment #10) > hey guys, do we need a try link/run for this change ? No. Thanks for asking!

Can we please? :P

Sorry, I'm clearly not nearly well-caffeinated enough yet today :) https://hg.mozilla.org/integration/fx-team/rev/ea8e8684eea3

Comment on attachment 8420308 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Needed for bug 985509. User impact if declined: Wrong open state on Win 7 Testing completed (on m-c, etc.): On m-c Risk to taking this patch (and alternatives if risky): Low, css only String or IDL/UUID changes made by this patch: None

[disabled=true] when all the other rules are just simply [disabled]?

https://hg.mozilla.org/releases/mozilla-aurora/rev/a9b03fc36271 https://hg.mozilla.org/releases/mozilla-beta/rev/ebf5c0367290

This issue is verified fixed on Windows 7 64-bit, using: * Firefox 30 Beta 6 build2 (Build ID: 20140520115057) [1], * Aurora 31.0a2 (2014-05-20) [2], * Nightly 32.0a1 (2014-05-20) [3]. One note though, the selected state of the star button from Aurora looks slightly different than the one from beta or nightly, screenshot here [4]. Any thoughts on this? [1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 [2] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 [3] Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:32.0) Gecko/20100101 Firefox/32.0 [4] http://goo.gl/FOoK4B

There's still one glaring problem: the right hand border of the depressed button is too light. Compare this with the mousedown state of the bookmark button and the opened state of the bookmarks menu button.

[1] bookmark button, mousedown [2] bookmark button, opened [3] bookmarks menu button, mousedown/opened [4] bookmark button, hover Problems: [1] has an unnecessary border-radius on topright and bottomright (compared to [4] and [3]) [2] has a missing border-right, causing the separator to show through.

(In reply to henryfhchan from comment #22) > Created attachment 8426145 [details] > bug-998687.png > > [1] bookmark button, mousedown > [2] bookmark button, opened > [3] bookmarks menu button, mousedown/opened > [4] bookmark button, hover > > Problems: > [1] has an unnecessary border-radius on topright and bottomright (compared > to [4] and [3]) > [2] has a missing border-right, causing the separator to show through. Please file a new bug. We have the same bug on Windows 8.