1742801 - Clicking a link in Gmail and slack.com inline text invokes popup blocker in Nightly (20211124035313)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Edgar Chen [:edgar]]

It looks like gmail also dispatch an untrusted event to open a new tab and the user activation has been consumed by ClickHandlerChild? I am investigating.

Comment 1

[Edgar Chen [:edgar]]

Yeah, gmail preventDefault the original click event, and dispatches an untrusted one.
And the user activation has been consumed by ClickHandlerChild, but ClickHandlerParent doesn't handle the click, https://searchfox.org/mozilla-central/rev/aa8c75b83f636948f708986173965c84cae8c25f/browser/actors/ClickHandlerParent.jsm#96-99.

One possible solution is to consume the activation only if ClickHanlder would handle the click, but I am not sure if it is possible to access whereToOpenLink in the content process? Gijs, any ideas? Thanks!

Comment 2

[Edgar Chen [:edgar]]

Attachment: test.html

Comment 3

[:Gijs (he/him)]

(In reply to Edgar Chen [:edgar] from comment #1)

One possible solution is to consume the activation only if ClickHanlder would handle the click, but I am not sure if it is possible to access whereToOpenLink in the content process? Gijs, any ideas? Thanks!

It isn't right now but we can fix this. Thanks for the testcase, I'll take a look.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1742801 - do not consume the user gesture from ClickHandlerChild if ClickHandlerParent will ignore the click anyway, r?mhowell,mtigley,edgar

This commit does a couple of things:

• move whereToOpenLink and getRootEvent implementations into BrowserUtils,
  so they can be used from the child process.
• forward callers in utilityOverlay.js to the BrowserUtils ones
  (bug 1742889 will get rid of the forwarding and update all the callers;
  we might be able to get this and bug 1739929 into beta if risk is low
  enough, and touching a bunch of extra files really doesn't help with
  that)
• move the lazy-load of BrowserUtils from browser.js to utilityOverlay.js
  This is safe because everywhere that loads browser.js also loads
  utilityOverlay.js. It's needed because there are some places that use
  utilityOverlay.js but not browser.js, and so now they need access to
  BrowserUtils.jsm.
• use whereToOpenLink to determine if we should avoid consuming the transient
  user gesture activation in the child click handling code.
• add an automated test based on the testcase in the bug.

When working on this, I initially put the check using whereToOpenLink in
the toplevel of the function, and then when I ran places test to check that
I hadn't broken any places consumers of whereToOpenLink or getRootEvent,
realized that I had broken browser_markPageAsFollowedLink.js, because it
relies on "normal" (ie no modifier key, left button) link clicks making it
to ClickHandlerParent.jsm . I filed bug 1742894 about this. I've not tried
to fix that here, instead I've tried to ensure that paths through this
function are as untouched as possible while still fixing bug 1739929 and
bug 1742801.

Comment 6

[Edgar Chen [:edgar]]

Attachment: test2.html

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/521231345acc
do not consume the user gesture from ClickHandlerChild if ClickHandlerParent will ignore the click anyway, r=edgar

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/521231345acc

Comment 9

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

Gijs says in bug 1739929 comment 32 that if we uplift that bug's Gmail fix to ESR 91, we'll also need to uplift this bug's fix.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1739929#c32, it sounds like the target for this is the 91.6esr release.

Comment 11

[Alexandru Trif, Desktop Test Engineering [:atrif] [PTO 15-29 Aug, please ni? :mboldan]]

Reproduced the initial issue with Firefox 96.0a1 (20211124155224) on Windows 10x64 and the attached test cases.
The issue is verified fixed with Firefox 96.0b4 (20211213214351) and 97.0a1 (20211213214351) on Windows 10x64, macOS 10.15 and Ubuntu 20.04. The pop-ups are no longer displayed when clicking links from the attached test cases.

Comment 12

[:Gijs (he/him)]

Comment on attachment 9252409 [details]
Bug 1742801 - do not consume the user gesture from ClickHandlerChild if ClickHandlerParent will ignore the click anyway, r?mhowell,mtigley,edgar

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes cmd-click / middle click on links in gmail (together with bug 1739929)
• User impact if declined: see above
• Fix Landed on Version: 96
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Has had a bunch of bake time, and we've already identified the one regression it's caused (bug 1742801).

Known risk here is the perf regression in bug 1745720. We won't be uplifting that fix considering the risk involved. The overall perf impact should be minimal, and the overall speedometer result won't move.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9252409 [details]
Bug 1742801 - do not consume the user gesture from ClickHandlerChild if ClickHandlerParent will ignore the click anyway, r?mhowell,mtigley,edgar

Approved for 91.6esr.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/6f820b5d5d9b

Comment 15

[Alexandru Trif, Desktop Test Engineering [:atrif] [PTO 15-29 Aug, please ni? :mboldan]]

Verified fixed with Firefox 91.6.0esr (20220131231250) on Windows 10x64, macOS 10.15, and Ubuntu 20.04. The pop-ups are no longer displayed when clicking links from the attached test cases.