Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

not prevent click events since Firefox 96 #3985

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

gdh1995
Copy link
Contributor

@gdh1995 gdh1995 commented Jan 13, 2022

It seems Firefox 96 allows extensions to trigger native click behaviors.
See #3964 for reports.

Although this is not listed in https://www.mozilla.org/en-US/firefox/96.0/releasenotes/,
I think it's safe enough to skip preventing since Firefox 96.

This will fix #3964, fix #3979, fix #3986 and fix #3579 (comment)

====

Updated on 2022/08/09:

I was still curious about why it's Firefox 96 which began to behave differently, so according to the clue about Firefox 91.6.0esr in #4000, today I compared 91.5.1esr and 91.6.0esr and successfully found the root difference.

It's the bug fix of https://bugzilla.mozilla.org/show_bug.cgi?id=1739929 from Firefox team, and it's imported to make GMail work on it. Sadly to learn that complaints from us extension developers didn't make any contribution in it.

On Firefox 91.0, there're two chains handling click events on <a>:

  1. The JS call enters Node::DispatchEvent
  2. go to normal event-dispatching process
  1. call HTMLAnchorElement::PostHandleEvent -> nsGenericHTMLElement::PostHandleEventForLinks
  2. call Element::PostHandleEventForLinks
  • branch 4.a:
      1. if a click event is left-click and it has no modifiers, then Dispatch a non-trustable DOMActivate event
      1. if the DOMActivate event is prevented, abort
      1. enter Element::PostHandleEventForLinks again
      1. call nsContentUtils::TriggerLink(..., click=true, trusted=false)
      1. in nsDocShell::OnLinkClick creates an OnLinkClickEvent instance and dispatches it
      1. nsDocShell::OnLinkClickSync calls nsDocShell::InternalLoad, and go to nsGlobalWindowOuter::OpenInternal
      1. RevisePopupAbuseLevel(openAbused) calls IsPopupAllowed -> CanShowPopup
      1. WindowContext::CanShowPopup returns ! getPerf("dom.disable_open_during_load")
      1. the DOMActivate event and click events are marked eConsumeNoDefault
  • branch 4.b:
      1. if a click event has modifiers, then return and continue dispatching
  1. if result is eConsumeNoDefault, then stop
  2. call "JS actors" including ClickHandlerChild, and it sends a json named Content:Click with modifier information
    1. on a Firefox before v91.6 and v96, the event is ignored by configuration in BrowserGlue.jsm:ClickHandler
    1. as for Firefox 96 and 91.6, the above bug fix added wantUntrusted: true into ClickHandler, so events work

On Firefox 96+, there's an extra logic:

  1. if a event is dispatched during a listener of browser.runtime.Port::onMessage, then push openAllowed or openControlled
    1. Unfortunately I didn't find the detailed code line
  1. then during a normal event-dispatching process, although GetEventPopupControlState returns openAbused, it won't be pushed when building AutoPopupStatePusher
  2. so RevisePopupAbuseLevel in the above 4.a.7 will return allowing.
  3. then Firefox won't block "a plain click on <a target=_blank href> when dom.disable_open_during_load is true"

====

The dom.disable_open_during_load can be enabled by Privacy & Security -> Block pop-up windows on about:preferences.

It seems Firefox 96 allows extensions to trigger native click behaviors.
See philc#3964 for reports.

Although this is not listed in https://www.mozilla.org/en-US/firefox/96.0/releasenotes/,
I think it's safe enough to skip preventing since Firefox 96.
@philc
Copy link
Owner

philc commented Jan 15, 2022 via email

Copy link

@djpowers djpowers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine to me. I also pulled this down and can confirm that it resolved the issue for me in Firefox 97.

@philc philc merged commit f9cb121 into philc:master Jan 20, 2022
@philc
Copy link
Owner

philc commented Jan 20, 2022

@gdh1995 MVP -- thanks Dahan!

philc added a commit that referenced this pull request Jan 20, 2022
@gdh1995 gdh1995 deleted the not-prevent-click-on-ff-96 branch January 20, 2022 16:45
@ic-768
Copy link

ic-768 commented Feb 12, 2022

This is still happening for me in firefox v.96 and vimium 1.67.1

@gdh1995
Copy link
Contributor Author

gdh1995 commented Feb 14, 2022

@ic-768 Have you enabled privacy.resistFingerprinting on about:config? It will cause some issues and #4000 may fix it.

@xbsuser5143
Copy link

There's no problem in Firefox 97.0.1 and Vimium 1.67.1, but the issue (using F, af, or et opens links twice) is still happening in Firefox ESR 91.6.0 and Vimium 1.67.1, even when privacy.resistFingerprinting is disabled.

@gdh1995
Copy link
Contributor Author

gdh1995 commented Feb 22, 2022

@xbsuser5143 Confirmed. This has only occurred since Firefox 91.6.0 esr (but not 91.5.0), so I've updated #4000 to fix this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants