Closed Bug 1399172 Opened 7 years ago Closed 7 years ago

Doorhangers created with AppMenuNotifications don't have their default buttons highlighted

Categories

(Firefox :: Theme, defect)

57 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 ? fixed
firefox57 --- fixed

People

(Reporter: alexical, Assigned: alexical)

References

Details

(Keywords: regression)

Attachments

(2 files)

This is a regression due to 1377006.

This also affects the recently restyled Flash doorhanger, since it is temporarily borrowing styles from the PopupNotifications doorhangers (Bug 1398972 tracks migrating the Flash doorhanger fully to PopupNotifications.jsm.)
Comment on attachment 8907166 [details]
Bug 1399172 - Add buttonhighlight to update doorhangers

https://reviewboard.mozilla.org/r/178854/#review184244

Ah, indeed, I didn't consider these elements, I should have known it affects them. Thank you for catching that. Be sure to uplift this as I think the other patch got uplifted as well.
Attachment #8907166 - Flags: review?(jhofmann) → review+
Comment on attachment 8907167 [details]
Bug 1399172 - Add highlight styling to Flash notification

https://reviewboard.mozilla.org/r/178856/#review184246
Attachment #8907167 - Flags: review?(jhofmann) → review+
[Tracking Requested - why for this release]:
Makes the update UI confusing, very simple to fix.
This would need to land quickly (today) to make it into 56.
Flags: needinfo?(dothayer)
Keywords: checkin-needed
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc4449801edc
Add buttonhighlight to update doorhangers r=johannh
https://hg.mozilla.org/integration/autoland/rev/5b931afbbbde
Add highlight styling to Flash notification r=johannh
Keywords: checkin-needed
Comment on attachment 8907166 [details]
Bug 1399172 - Add buttonhighlight to update doorhangers

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1377006 added an option for not highlighting the default button in a doorhanger. The addition of this option caused the update doorhangers to not have their default buttons highlighted.
[User impact if declined]: The update doorhangers would have both buttons colored grey, rather than one of them being colored blue. This would be different from our other doorhangers and slightly confusing to  users.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: It only affects update doorhanger styling, and that styling is already broken without it.
[String changes made/needed]: N/A
Flags: needinfo?(dothayer)
Attachment #8907166 - Flags: approval-mozilla-beta?
Comment on attachment 8907166 [details]
Bug 1399172 - Add buttonhighlight to update doorhangers

Polish for new doorhanger/tour UI, let's uplift for beta 12.
Attachment #8907166 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Doug Thayer [:dthayer] from comment #8)
> [Needs manual test from QE? If yes, steps to reproduce]: No
Marking this as qe-verify-, based on Doug's assessment on manual testing needs.
Flags: qe-verify-
Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.