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

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Doug Thayer [:dthayer] (he/him)]

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 1

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1399172 - Add buttonhighlight to update doorhangers

The highlight styling was recently added, causing update notification
buttons to both be styled as grey. This fixes that to style them as
blue again.

Review commit: https://reviewboard.mozilla.org/r/178854/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178854/

Comment 2

[Doug Thayer [:dthayer] (he/him)]

Attachment: Bug 1399172 - Add highlight styling to Flash notification

The Flash (plugin) notification buttons have been restyled
temporarily to make them in line with other doorhangers. There
is further work being done to use PopupNotifications.jsm fully
for these notifications, rather than PopupNotifications + a custom
XBL binding. However, this won't make it for 57, so this change
fixes the temporary restyle to ensure that the default button
is blue.

Review commit: https://reviewboard.mozilla.org/r/178856/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178856/

Comment 3

[Johann Hofmann [:johannh]]

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.

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8907167 [details]
Bug 1399172 - Add highlight styling to Flash notification

https://reviewboard.mozilla.org/r/178856/#review184246

Comment 5

[Johann Hofmann [:johannh]]

[Tracking Requested - why for this release]:
Makes the update UI confusing, very simple to fix.

Comment 6

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

This would need to land quickly (today) to make it into 56.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

Comment 8

[Doug Thayer [:dthayer] (he/him)]

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

Comment 10

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8907166 [details]
Bug 1399172 - Add buttonhighlight to update doorhangers

Polish for new doorhanger/tour UI, let's uplift for beta 12.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/88a62be317ee
https://hg.mozilla.org/releases/mozilla-beta/rev/a0d1ab4aa8c2

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/fc4449801edc
https://hg.mozilla.org/mozilla-central/rev/5b931afbbbde

Comment 13

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

(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.