1606839 - Use new user activation API for notification permission dialog

Status: VERIFIED FIXED

(Core :: DOM: Push Subscriptions, task, P1)

Description

[Anne (:annevk)]

As Edgar wrote in bug 1591718 comment 11:

I expect that replacing the UserActivation::IsHandlingUserInput() call in ContentPermissionRequestBase with new API in the document should just work.

This would likely fix an active regression on twitter.com (please verify), maybe elsewhere, and is in general a thing we want to be doing.

This probably does not warrant an intent to ship, if indeed it's as straightforward as it seems.

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1606839 - Use new user activation API for guarding notification permission prompts. r=edgar,Ehsan

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/209adca8625c
Use new user activation API for guarding notification permission prompts. r=edgar

Comment 3

[Arthur Iakab [arthur_iakab]]

Backed out changeset 209adca8625c (bug 1606839) for multiple failures on browser_permissions_handling_user_input.js

Backout revision https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=284851116&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=83736c0c967d71d6d5c90691c83e2fb31c8aa7fd

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=209adca8625ca6111cc5e1a4c22a1b81162064f6

Failures logs: - bc2 https://treeherder.mozilla.org/logviewer.html#?job_id=284850396&repo=autoland
- c2 https://treeherder.mozilla.org/logviewer.html#?job_id=284849976&repo=autoland
- ss https://treeherder.mozilla.org/logviewer.html#?job_id=284866347&repo=autoland

Johann can you please take a look?

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/488a0a910f01
Backed out changeset 209adca8625c for multiple failures on browser_permissions_handling_user_input.js

Comment 5

[Johann Hofmann [:johannh]]

Edgar, I would have expected E10SUtils.wrapHandlingUserInput to set UserActivation::IsHandlingUserInput() like it does with isHandlingUserInput but it seems like that's not the case yet. Is there already a mechanism to artificially set user activation for tests or would you like me to add one?

Comment 6

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Johann Hofmann [:johannh] from comment #5)

Is there already a mechanism to artificially set user activation for tests or would you like me to add one?

There is a chrome only api in document, you could use it to set user activation for tests.

Comment 7

[Johann Hofmann [:johannh]]

Ah, thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d48d8dbcbbea
Use new user activation API for guarding notification permission prompts. r=Ehsan,edgar

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d48d8dbcbbea

Comment 10

[Johann Hofmann [:johannh]]

Comment on attachment 9119765 [details]
Bug 1606839 - Use new user activation API for guarding notification permission prompts. r=edgar,Ehsan

Beta/Release Uplift Approval Request

• User impact if declined: Well-meaning sites which fail to capture user interaction correctly do not get the more prominent notification permission prompts, e.g. twitter.com
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See bug 1591718
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a simple change that simply loosens some restrictions around notification prompting, with tests.
• String changes made/needed: None

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9119765 [details]
Bug 1606839 - Use new user activation API for guarding notification permission prompts. r=edgar,Ehsan

Fixes a regression causing some push notification prompts to not appear on popular sites like Twitter. Approved for 73.0b7.

Comment 12

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/ff4054a708c6

Comment 13

[Alin Ilea, Desktop Test Engineering]

Reproduced the initial issue using release version 72.0.1
Verified - Fixed in latest Beta build 73.0b7 (Build id: 20200118195856) and latest Nightly build 74.0a1 (Build id: 20200119213718) using Windows 10, Ubuntu 18.04 and Mac OS 10.15