983801 - Australis - Remove border-radius on the bookmarks panel submenus on Windows 8

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Comment 1

[Tim Nguyen :ntim]

This bug is for the bookmarks panel submenus.

Comment 2

[Mike de Boer [:mikedeboer]]

Tim, can you post a screenshot to explain exactly what you mean?

Comment 3

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #2)
> Tim, can you post a screenshot to explain exactly what you mean?

http://images.devs-on.net/Image/m8B5HJfLanLVkbUT-Region.png

Comment 4

[Tim Nguyen :ntim]

Attachment: Patch 1 : Remove border-radius from Bookmarks panel submenus on Windows 8 r=gijs

Comment 5

[:Gijs (he/him)]

Comment on attachment 8395283 [details] [diff] [review]
Patch 1 : Remove border-radius from Bookmarks panel submenus on Windows 8 r=gijs

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

This isn't quite right, because if we're on aero basic or windows classic on win7, we'd still want rounded corners (we use them everywhere, not just for the bookmarks panel), and this selector doesn't work for that case. Equally, we can't just select for win7 or winxp, because we'd leave out all the poor vista users. Having all three selectors seems a bit ugly to me, so I wonder if there's a better way.

I'm not sure how best to do what you're aiming for, because I'm not sure where the panel corner styling is done for the rest of the panels; Mike de Boer might be a better reviewer. Have you looked at how we do the styling for the main bookmarks panel or the main menu panel? That might give a clue.

Comment 6

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #5)
> the bookmarks panel), and this selector

Err, obviously I meant media query. It's late here.

Comment 7

[Tim Nguyen :ntim]

Attachment: Patch 2 - Remove border-radius from bookmarks panel submenus on Windows 8

Addressed gijs review comments.

Comment 8

[Mike de Boer [:mikedeboer]]

Comment on attachment 8395355 [details] [diff] [review]
Patch 2 - Remove border-radius from bookmarks panel submenus on Windows 8

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

This is kinda the right direction, but not quite ;)

What you need to do here is add the selector `#BMB_bookmarksPopup menupopup[placespopup=true] > hbox` to the list of selectors that set `border-radius: 0` at the bottom of the panelUIOverlay.css file. While you busy there, can you merge the two blocks into one?

Comment 9

[Mike de Boer [:mikedeboer]]

And Tim, I think it's just awesome that you're doing patches now! Keep it up!!!

Comment 10

[Tim Nguyen :ntim]

Attachment: Patch 3 : Remove border-radius from Bookmarks panel submenus on Windows 8

Addressed Mike's comments. Also combined 2 css rules since they were about the same thing.

Comment 11

[Mike de Boer [:mikedeboer]]

Comment on attachment 8397051 [details] [diff] [review]
Patch 3 : Remove border-radius from Bookmarks panel submenus on Windows 8

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

LGTM! Thanks!

Comment 12

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

https://hg.mozilla.org/integration/fx-team/rev/48a5e153526a

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/48a5e153526a

Comment 14

[Tim Nguyen :ntim]

Comment on attachment 8397051 [details] [diff] [review]
Patch 3 : Remove border-radius from Bookmarks panel submenus on Windows 8

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis 
User impact if declined: rounded border on the bookmarks panel submenus on Windows 8
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 15

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4a0223fb8ac0

Comment 16

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-beta/rev/575f9faf293a

Comment 17

[Cornel Ionce [:noni] [Hubs QA]]

Verified on Windows 8 x64 and Windows 8.1 x64 using Firefox 30 (build ID: 20140605174243) and Firefox 31 beta 5 (build ID: 20140626181429) and the border-radius is no longer displayed.

Marking issue as verified.