UITour: Make the highlight effect a circle on the bookmarks combo button

VERIFIED FIXED in Firefox 29

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: bwinton, Assigned: bwinton)

Tracking

30 Branch
Firefox 30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29 verified, firefox30 verified)

Details

(Whiteboard: [Australis:P4+])

Attachments

(1 attachment, 1 obsolete attachment)

The previous patch was meant to make the bookmarks combo button have a circular highlight (and I _swear_ it did when I tested it), but apparently on nightly it doesn't anymore, so this patch is here to give me a chance to fix that.
Posted patch bug983653.patch (obsolete) — Splinter Review
The bookmarks button now has a ratio of 2.6363636363636362 (instead of the 2.01) it had when I wrote the patch), so I think "3" will give us enough room to handle any future padding changes.
Assignee: nobody → bwinton
Attachment #8391202 - Flags: ui-review?(shorlander)
Attachment #8391202 - Flags: review?(MattN+bmo)
Comment on attachment 8391202 [details] [diff] [review]
bug983653.patch

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

Looks good to me!
Attachment #8391202 - Flags: ui-review?(shorlander) → ui-review+
Comment on attachment 8391202 [details] [diff] [review]
bug983653.patch

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

Holly is also fine with trying this.

::: browser/modules/UITour.jsm
@@ +810,5 @@
>        let highlightWidth = targetRect.width;
>        let minDimension = Math.min(highlightHeight, highlightWidth);
>        let maxDimension = Math.max(highlightHeight, highlightWidth);
>  
>        // If the dimensions are within 110% of each other (to include the bookmarks button),

You forgot to update the comment again :)
Attachment #8391202 - Flags: review?(MattN+bmo) → review+
Fixed the comment.  :)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): (I can't find it, but something changed the padding or margin on the bookmarks button.)
User impact if declined: Less noticeable bookmarks highlight in the UI tour.
Testing completed (on m-c, etc.): Manual.  Also the video in bug 983737.
Risk to taking this patch (and alternatives if risky): Very low.
String or IDL/UUID changes made by this patch: None.
Attachment #8391202 - Attachment is obsolete: true
Attachment #8391334 - Flags: ui-review+
Attachment #8391334 - Flags: review+
Attachment #8391334 - Flags: approval-mozilla-aurora?
I will approve it once it is in m-c.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/a6195a333647
Status: NEW → ASSIGNED
Keywords: checkin-needed
Summary: UI Tour: Fix shape of UI highlight effect on Bookmarks combo button. → UITour: Make the highlight effect a circle on the bookmarks combo button
Whiteboard: [Australis:P4+] → [Australis:P4+][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/a6195a333647
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4+][fixed-in-fx-team] → [Australis:P4+]
Target Milestone: --- → Firefox 30
Attachment #8391334 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed using the following environment:
Firefox Aurora 29.0a2
Build Id:20140317004002
OS: Win 8.1 x 32, Mac Os x 10.8.5, Ubuntu 13.04 x64
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.