Closed Bug 983801 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Theme, defect)

30 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- fixed
firefox30 --- verified
firefox31 --- verified

People

(Reporter: ntim, Assigned: ntim)

References

Details

(Whiteboard: [Australis:P-])

Attachments

(1 file, 2 obsolete files)

No description provided.
Blocks: theme-win8
Whiteboard: [Australis:P-]
This bug is for the bookmarks panel submenus.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Tim, can you post a screenshot to explain exactly what you mean?
Flags: needinfo?(ntim007)
(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
Flags: needinfo?(ntim007)
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
Attachment #8395283 - Flags: review?(gijskruitbosch+bugs)
Attachment #8395283 - Attachment description: bug_983801.patch → Patch 1 : Remove border-radius from Bookmarks panel submenus on Windows 8
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.
Attachment #8395283 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #5)
> the bookmarks panel), and this selector

Err, obviously I meant media query. It's late here.
Addressed gijs review comments.
Attachment #8395283 - Attachment is obsolete: true
Attachment #8395355 - Flags: review?(mdeboer)
Summary: Australis - Remove border-radius on the bookmarks submenus on Windows 8 → Australis - Remove border-radius on the bookmarks panel submenus on Windows 8
Attachment #8395283 - Attachment description: Patch 1 : Remove border-radius from Bookmarks panel submenus on Windows 8 → Patch 1 : Remove border-radius from Bookmarks panel submenus on Windows 8 r=gijs
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?
Attachment #8395355 - Flags: review?(mdeboer) → feedback+
And Tim, I think it's just awesome that you're doing patches now! Keep it up!!!
Addressed Mike's comments. Also combined 2 css rules since they were about the same thing.
Attachment #8397051 - Flags: review?(mdeboer)
Attachment #8395355 - Attachment is obsolete: true
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!
Attachment #8397051 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/integration/fx-team/rev/48a5e153526a
Keywords: checkin-needed
Whiteboard: [Australis:P-] → [Australis:P-][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/48a5e153526a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P-][fixed-in-fx-team] → [Australis:P-]
Target Milestone: --- → Firefox 31
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
Attachment #8397051 - Flags: approval-mozilla-beta?
Attachment #8397051 - Flags: approval-mozilla-aurora?
Attachment #8397051 - Flags: approval-mozilla-beta?
Attachment #8397051 - Flags: approval-mozilla-beta+
Attachment #8397051 - Flags: approval-mozilla-aurora?
Attachment #8397051 - Flags: approval-mozilla-aurora+
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.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.