1224738 - Notification Settings does not work in MAC

Status: VERIFIED FIXED

(Core :: DOM: Push Subscriptions, defect)

Description

[Abe - QA (:Abe_LV)]

User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Firefox/45.0 
Nightly 45.0a1   Build ID:20151113030238


1. Access https://pdehaan.github.io/push-notification-test/
2. Click the lock icon and set the Permission option to Always Ask
3. Click the pop Notification button
4. Verify the door hanger displays
5. Set the option to Always Receive Notifications
6. Verify the pop-up dialog for the notification is displayed
7. Click the Settings icon on the pop-up
8. Select the option Notification Settings
9. Verify the current window opens a new tab for Options (about:preferences#content) 

Actual Result:
Nothing happens

Expected Result:

New tab should display options (about:preferences#content)

Comment 1

[Lina Butler [:lina]]

Attachment: MozReview Request: Bug 1224738 - Fix alternate action index getter name on OS X. r?MattN

Bug 1224738 - Fix alternate action index getter name on OS X. r?MattN

Comment 2

[Lina Butler [:lina]]

Argh, I introduced this regression in bug 1220337. Sorry!

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8688077 [details]
MozReview Request: Bug 1224738 - Fix alternate action index getter name on OS X. r?MattN

https://reviewboard.mozilla.org/r/25285/#review22767

Comment 4

[Lina Butler [:lina]]

https://hg.mozilla.org/integration/fx-team/rev/996fabe1edaa17ae416baf84089baa2c50076707
Bug 1224738 - Fix alternate action index getter name on OS X. r=MattN

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/996fabe1edaa

Comment 6

[Lina Butler [:lina]]

Comment on attachment 8688077 [details]
MozReview Request: Bug 1224738 - Fix alternate action index getter name on OS X. r?MattN

Approval Request Comment
[Feature/regressing bug #]: Bug 1220337.
[User impact if declined]: Bug 1220337 broke the "disable notifications" and "notification settings" actions on OS X.
[Describe test coverage new/current, TreeHerder]: Manually tested using the steps to reproduce on OS X 10.8, 10.9, and 10.10, with e10s disabled and enabled.
[Risks and why]: Clicking on the notification actions in OS X 10.9 and later currently does nothing on Aurora. This restores the broken functionality.
[String/UUID change made/needed]: None.

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Abi, could you please verify that this issue is fixed as expected in the latest Nightly build? Thanks!

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8688077 [details]
MozReview Request: Bug 1224738 - Fix alternate action index getter name on OS X. r?MattN

Fixes a recent regression, we need to take it. Aurora44+

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Kit, is it really hard to add automated tests for this? It would be much easier for us to catch regressions like this if we had some automated tests running regularly. What do you think?

Comment 10

[Lina Butler [:lina]]

(In reply to Ritu Kothari (:ritu) from comment #9)
> Kit, is it really hard to add automated tests for this?

I'm not sure how much of this we can automate. :MattN and I found a way to control notifications with AppleScript (http://macosxautomation.com/mavericks/notifications/01A.html), but that requires changing Accessibility prefs on the test machine. It's definitely hard, but it does look possible.

> It would be much easier for us to catch regressions like this if we had some
> automated tests running regularly.

I agree. :-)

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ccd22a68675b

Comment 12

[Abe - QA (:Abe_LV)]

User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Firefox/45.0 
Nightly 45.0a1 Build ID:20151124030553

The issue is resolved. It is working fine.

Comment 13

[Abe - QA (:Abe_LV)]

It is verified and the issue is resolved.
Changing the status from Resolved fixed to Verified: Fixed.