Closed
Bug 1401009
Opened 7 years ago
Closed 7 years ago
disableHighlight in PopupNotifications doesn't work properly for more than one button
Categories
(Toolkit Graveyard :: Notifications and Alerts, defect, P3)
Toolkit Graveyard
Notifications and Alerts
Tracking
(firefox57 verified, firefox58 verified)
VERIFIED
FIXED
mozilla58
People
(Reporter: mkaply, Assigned: mkaply)
References
Details
Attachments
(4 files)
The new disableHighlight feature added in bug 1377006 doesn't work properly when there is more than one button. See attachment. Solution should be to add a border, but it will be on either the left or right depending on Windows/Mac.
Updated•7 years ago
|
Priority: -- → P3
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
Because the patch is platform specific (left or right border), I had to put it in the platform popupnotification css files. Not sure if there is a way to do this in a platform independent way.
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8911366 [details] Bug 1401009 - Add border between notification buttons when highlight is disabled. https://reviewboard.mozilla.org/r/182840/#review188738 All these rules should be border-inline-{start,end} instead, to account for rtl. Putting them inside the platform specific files sounds fine to me.
Attachment #8911366 -
Flags: review?(jhofmann) → review-
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8911366 [details] Bug 1401009 - Add border between notification buttons when highlight is disabled. https://reviewboard.mozilla.org/r/182840/#review188948 Thank you!
Attachment #8911366 -
Flags: review?(jhofmann) → review+
Comment 6•7 years ago
|
||
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 3 changes to 3 files remote: remote: remote: ************************** ERROR **************************** remote: Rev d9274643bde4 needs "Bug N" or "No bug" in the commit message. remote: Michael Kaply <mozilla@kaply.com> remote: Bug #1401009 - Add border between notification buttons when highlight is disabled. r=johannh remote: remote: MozReview-Commit-ID: 4YGdUZstiDk remote: ************************************************************* remote: remote: remote: transaction abort! remote: rollback completed remote: pretxnchangegroup.c_commitmessage hook failed abort: push failed on remote
Comment hidden (mozreview-request) |
Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/a44a81c459ab Add border between notification buttons when highlight is disabled. r=johannh
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a44a81c459ab
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8911366 [details] Bug 1401009 - Add border between notification buttons when highlight is disabled. Approval Request Comment [Feature/Bug causing the regression]: Bug 1377006 [User impact if declined]: Vertical line missing in popup notification. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Add a search engine using the new is_default API [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Changes CSS for this one case [String changes made/needed]: None This is a purely visual fix that would be nice to have. Very low risk.
Attachment #8911366 -
Flags: approval-mozilla-beta?
Comment 11•7 years ago
|
||
Comment on attachment 8911366 [details] Bug 1401009 - Add border between notification buttons when highlight is disabled. Polish the UI, taking it. Should be in 57b4, gtb later today
Attachment #8911366 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/2399e64d6af2
status-firefox57:
--- → fixed
Comment 13•7 years ago
|
||
This bug is verified on Firefox 58.0a1 (20170927100120) under Windows 10 64-bit and Mac OS X 10.12.3. Please see the attached screenshot.
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Comment 14•7 years ago
|
||
Let's make sure this works as intended on Beta 57 as well.
Flags: qe-verify+
Comment 15•7 years ago
|
||
This bug is verified on Firefox 57.0b13 (20171030163911) under Windows 10 64-bit and Mac OS X 10.12.3. Please see the attached screenshot.
Updated•7 years ago
|
Updated•10 months ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•