Closed Bug 1379466 Opened 7 years ago Closed 6 years ago

Inconsistent popup-blocker behavior with document.addEventListener document.body.addEventListener onclick auxclick

Categories

(Core :: DOM: Events, defect, P2)

Product:
Core
Component:
DOM: Events
Version:
54 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: u598317, Assigned: Kwan)

References

(Regressed 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(6 files, 11 obsolete files)

Bug 1379466 - Add auxclick to mouse synthesis test code.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1379466 - Set EventPopupControlState of auxclick as 'openControlled'.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1379466 - Use auxclick event to trigger new tab on middle click.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1379466 - Make editor listen for auxclick mouse events.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1379466 - Stop dispatching click events for non-primary mouse clicks on the web.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1379466 - Add override pref to restore legacy non-primary click dispatch on specific domains.
47 bytes, text/x-phabricator-request
Details | Review
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0 Build ID: 20170616092053 Steps to reproduce: Depending on where addEventListener() is attached and which event is acted on, a new tab initiated on user-click may be erroneously or unreliably blocked by the popup blocker. I've included the exact test.html file I'm using below showing example (minimised) code, and comments further explaining under exactly which circumstances the faults occur. It's 100% reproduced every time. <!DOCTYPE HTML><head><meta charset="utf-8"><title>TEST</title></head><body> <a href="http://example.com">http://example.com (middle-click me)</a> <script> function check_click(e) { if (e.which == 3) return; //ignore right-mouse-button if (e.target.tagName !== "A") return; //do nothing if it's not a link e.stopPropagation(); e.preventDefault(); if (e.which == 1) { //left-mouse-button disabled merely for the sake of this example alert('Click canceled. I wrote "middle-click me" ... please :-)'); return; } //Enable one or the other to illustrate the issues noted below, and //to prevent two tabs being opened: if (e.type == "click") return; //if (e.type == "auxclick") return; window.open(e.target.href, "_blank"); } //Enable one or the other below to see the issues noted: //These fail to open a new tab and show the popup blocker alert, regardless //of whether we act on "click" or "auxclick": document.addEventListener("click", check_click, true); document.addEventListener("auxclick", check_click, true); //These work properly only if acting on "auxclick". If acting on "click", then //it opens the new tab but still shows the popup blocker alert! //document.body.addEventListener("click", check_click, true); //document.body.addEventListener("auxclick", check_click, true); </script> </body> </html> Actual results: Depending on the exact code used, the popup blocker erroneously triggers, or it triggers (showing the alert) but the "popup" still occurs. Please refer to the details in the source-code comments above, which explain the faults in context (better than I can explain without the accompanying code). The fault is present regardless of operating-system or system configuration (I've tested under Debian/Linux and Windows 7 and the fault is the same). Expected results: None of the new tabs created on the example page should be blocked. They are all initiated upon a user-click. In case it's any help, testing in Chromium confirms that it works 100% in there as expected.
There's a further related inconsistency which I noticed after adding this to the top of the function: console.log(e.type); Upon pressing the middle mouse button: * When using document.addEventListener(), I noticed that "click" and "auxclick" are given. * But when using document.body.addEventListener(), only "auxclick" is given. (It's preferable that only auxclick is given - regardless of where the listener is attached, and this would also be consistent with Chromium.) Knowing this now, and after further experimentation, I'm seeing that new tabs are always being erroneously blocked.
Component: Untriaged → DOM: Events
Product: Firefox → Core
Hi Ben, would you please take a look at this?
Flags: needinfo?(btian)
Sure. Take it.
Assignee: nobody → btian
Flags: needinfo?(btian)
Two problems are described here: 1) |document.addEventListener| behavior differs from |document.body.addEventListener| one 2) New tab creation should not be blocked by popup blocker === Note the latest Nightly behavior on Mac is as following: document.addEventListener - click => popup block alert - auxclick => popup block alert + new tab opened document.body.addEventListener - click => new tab opened - auxclick => popup block alert + new tab opened
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Ben Tian [:btian] from comment #4) > Two problems are described here: > 1) |document.addEventListener| behavior differs from > |document.body.addEventListener| one click event is fired for bug 1304044 comment 2.
(In reply to Ben Tian [:btian] from comment #5) > click event is fired for bug 1304044 comment 2. Attach a patch that removes non-primary clicks. Verified with auxclick web platform test [1] and checking on try [2]. [1] https://w3c-test.org/uievents/auxclick/auxclick_event-manual.html [2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=26ed15c83f51cb917b194c716d6c826392b95e78
(In reply to Ben Tian [:btian] from comment #5) > click event is fired for bug 1304044 comment 2. Olli, do you think we can remove non-primary clicks now? If so I'll prepare a patch for review.
Flags: needinfo?(bugs)
Attach a patch that sets EventPopupControlState of auxclick as 'openControlled' to remove popup blocker alert of middle click. This patch is based on comment 6 patch.
Hi Ben. Thanks for working on this. I've just upgraded to the nightly build too, so I can hopefully have a similar test browser running. These are my results (pressing middle-mouse-button and using a custom handler for the resulting event(s) as detailed above): document.addEventListener - click => popup block alert [same as your result] - auxclick => popup block alert [different to your result] document.body.addEventListener - click => this event is not seen [different to you] - auxclick => popup block alert + new tab opened [same as you] HOWEVER... please note that in the last case, the reason why a new tab is opened is because e.stopPropagation() and e.preventDefault() are being ignored. This is easily proven by commenting out the line: window.open(e.target.href, "_blank"); -- no matter what I do, I cannot stop the browser doing its default action under these circumstances. So the popup blocker is actually preventing the new tab being opened by this line of JavaScript, but other browser code is opening a new tab as the default behavior anyway. So it seems that you now have another issue to look into here... Regarding removing non-primary clicks now -- the mere act of adding "auxclick" implies that "click" for non-primary buttons should no longer be given. Otherwise it's creating yet another/different thing to handle cross-browser since I'm wanting to perform my own custom actions for primary/non-primary buttons. I realize that "click" has probably been left in for potential existing code out there that might be using it for middle-button too (before "auxclick" was implemented), but if there is any such code, then they're going to have to update it eventually anyway now that auxclick (and no "click") is the new standard for middle/other buttons.
(In reply to Leslie from comment #9) I meant to mention that I'm testing under Linux. I don't have a Mac to test on.
(In reply to Ben Tian [:btian] from comment #7) > (In reply to Ben Tian [:btian] from comment #5) > > click event is fired for bug 1304044 comment 2. > > Olli, do you think we can remove non-primary clicks now? If so I'll prepare > a patch for review. I doubt we can do it so soon. Do other browsers still support click for non-primary button? (That has been inconsistent between browsers)
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #11) > Do other browsers still support click for non-primary button? Chrome/Chromium only gives auxclick (no longer issues click), and handles every possibility on my test page flawlessly. IMO, it's time to move Firefox forward too.
What about Safari or Edge?
(In reply to Olli Pettay [:smaug] from comment #13) > What about Safari or Edge? Checked Firefox, Chrome, and Safari on Mac and Edge on Win 10. The behaviors are quite inconsistent indeed ... | auxclick fired? | click fired? | ---------+-----------------+--------------+ Firefox | V | V | ---------+-----------------+--------------+ Chrome | V | X | ---------+-----------------+--------------+ Safari | X | V | ---------+-----------------+--------------+ Edge | X | X | === I'll keep checking 1) whether to remove popup block alert for middle click and 2) comment 9 preventDefault problem.
Thanks Ben. Also note the one case above where Firefox only issues auxclick for me. I also tested Edge and found that no click or auxclick is given. This is important for the purpose of this discussion because it makes Edge irrelevant to making any decision to change Firefox (to match Chrome) or not, since we're unable to see or alter Edge's default behavior via JS anyway. So if these are the only other browsers you're concerned about, then you'd only have Safari left to deal with. Though I'd like it to be resolved as I mentioned above, whichever way is decided on this issue isn't going to be a problem for me, since I already have a workaround which has to stay in for now to support old browsers anyway... it goes something like this (included mainly as an example for the benefit of anyone else who is also having to deal with this and happens to read this)... simplified code, supposing we're listening for click and auxclick and want to do something different for each: function handler(e) { //return here if some pre-conditions aren't met (e.g. ignore RMB and non-links); else... e.stopPropagation(); e.preventDefault(); if (e.type === "auxclick" || (e.which === 2 && !("onauxclick" in document))) { //do something for "auxclick"... return; } //else (e.type === "click") if (e.which === 2 && ("onauxclick" in document)) { //Firefox special-case: both are being issued for one thing, so cancel and ignore this "click"! return; } //do something for "click"... } Regarding the other issues, hopefully you manage to get your updates pushed through to Nightly so I can test them. Let me know if you want me to provide any further comments. Otherwise I'll leave it all in your hands. Cheers and thanks again for looking into it.
(In reply to Leslie from comment #9) > document.body.addEventListener > - click => this event is not seen [different to you] (In reply to Leslie from comment #15) > Thanks Ben. Also note the one case above where Firefox only issues auxclick > for me. Click event is not fired to document.body since |mNoContentDispatch| flag [1] is set for middle button [2]. Comment 14 table is based on document.addEventListener. [1] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/widget/BasicEvents.h#100-109 [2] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/dom/events/EventStateManager.cpp#4664-4666
(In reply to Ben Tian [:btian] from comment #16) > Click event is not fired to document.body since |mNoContentDispatch| flag > [1] is set for middle button [2]. On the contrary, click event is fired to document since |mForceContentDispatch| is set [1] and overrides |mNoContentDispatch|. The |mForceContentDispatch| was set as a hack to make middle mouse paste working also in Editor. [1] http://searchfox.org/mozilla-central/rev/09c065976fd4f18d4ad764d7cb4bbc684bf56714/dom/base/nsDocument.cpp#7989
See Also: → 1304044, 329119
Regarding comment 9 prevent default problem, the following is new tab opening flow triggered by 'click' event. Will confirm whether opening new tab is spec-defined default action for middle button, given 'click' is still kept for non-primary button. -- The new tab opening is triggered by 'click' event dispatching in content process, to topmost target's event listener in system group during capture phase [1]. The 'click' event is then handled by [2] and triggers new tab opening in [3] finally. [1] https://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/dom/events/EventDispatcher.cpp#488 [2] https://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/browser/base/content/content.js#585 [3] https://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/browser/modules/ContentClick.jsm#97
(In reply to Ben Tian [:btian] from comment #18) > The new tab opening is triggered by 'click' event dispatching in content > process, to topmost target's event listener in system group during capture > phase [1]. The 'click' event is then handled by [2] and triggers new tab > opening in [3] finally. Correction: 1) revise link [1] 2) should be last can handle target of the array in [1] (first to dispatch in capture phase), not 'topmost target' defined by spec. [1] https://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/dom/events/EventDispatcher.cpp#442 > [2] > https://searchfox.org/mozilla-central/rev/ > f0e4ae5f8c40ba742214e89aba3f554da0b89a33/browser/base/content/content.js#585 > [3] > https://searchfox.org/mozilla-central/rev/ > f0e4ae5f8c40ba742214e89aba3f554da0b89a33/browser/modules/ContentClick.jsm#97
So comment 9 preventDefault problem results from that: Click event triggers opening new tab, but it's not preventDefaulted since document.body doesn't receive click event (i.e., check_click is not called). While auxclick event is received but preventDefault() does nothing to it.
Back to original problems (comment 4): 1) To unify |document.addEventListener| and |document.body.addEventListener|, we can either - remove non-primary click as UI event spec, => cannot do it soon (comment 11) for browser inconsistency (comment 14) - OR dispatch click event to neither document nor document.body => but would break middle mouse paste in Editor (comment 17). 2) For popup block alert, I think it's reasonable if new tab is opened by script since script may open another link unknown to user, even though click is user-initiated.
Priority: -- → P2
(In reply to Ben Tian [:btian] from comment #21) > 2) For popup block alert, I think it's reasonable if new tab is opened by > script since script may open another link unknown to user, even though click > is user-initiated. Indeed, a different URL to what the user clicks on is used. To clarify why this ability/behavior is required (and important to be able to implement without hindrance), my use case (in brief) is that my colleagues and I are developing an interactive web-page analysis tool. The idea is that a user visits our website, enters a website URL, and we load that website within our wrapper. When the user clicks links in the supplied website, we need to intercept all of these clicks (hence the use of document.addEventListener): * left-click: our script continues their browsing session within our analysis wrapper for the new URL. * middle-click: our script opens a new tab for the new URL still within our wrapper-site; so if they click on: http://example.com/page2.html then our script opens a new tab such as: http://oursite-example.com/analyse#http://example.com/page2.html We now have everything working in Firefox except for the noted issues (although the user can allow popups with a minor one-time annoyance and proceed from there, but this looks somewhat unprofessional, and not everyone knows whether they should allow it or not, thus potentially breaking functionality). Most likely there are others needing this ability too.
See Also: → 968265
Assignee: ben.tian → nobody
Attachment #8888720 - Attachment is obsolete: true
Attachment #8944418 - Flags: feedback?(bugs)
So it turns out we can't entirely junk click for non-primary yet, since frontend code seems to be relying on it. What we can do though is stop sending it for web content. I think this would also allow ripping out all the NoContentDispatch and ForceContentDispatch machinery since AFAICT it only exists to let editor do middle mouse paste, thus resolving bug 329119 (and I have a patch on top of this lot to do so). Behaviour changes (that I know of, I might be missing some due to ignorance): * web no longer gets non-primary click events on input and textarea elements (only got them during capturing if middle-mouse pasting) * web no longer gets non-primary click events on window/document (again, only during capturing if middle-pasting into text field) * thus to block middle-click-open on links web will have to use preventDefault on auxclick instead of click * web can now block middle-click-open on links with individual element listeners rather than only a global one * the above obviously also applies to chrome code looking at web documents, but I'm not aware of any that uses that, and it can still do addSystemEventListener(TabChildGlobal etc.). Nothing seems to break: https://treeherder.mozilla.org/#/jobs?repo=try&revision=20bcdd9966de2b6b2f9dbbf2be27f3d7760ac88b * some chrome code will start getting non-primary clicks where it didn't before. The only instances I know of are the form autofill manage & edit dialogs in about:preferences, which are chrome:// protocol XHTML documents inside a <browser>. That'll need a small code change to stop buttons activating on non-primary. Of note: *chrome already unshipped non-primary click when they shipped auxclick, so if any websites do absolutely need to block middle-click-open, they should already be using auxclick anyway *editor/libeditor/tests/test_bug674770-1.html is already failing test verify: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5df084c2d61b7fb4792267a893e2667d84cf134 (all the blue is actually oranges, see bug 1431950) so if this lands that's expected orange.
Comment on attachment 8944418 [details] [diff] [review] 1 - Bug 1379466 - Set EventPopupControlState of auxclick as 'openControlled'. This is just Ben's existing patch rebased (hopefully correctly) around bug 918780.
Attachment #8944418 - Flags: feedback?(bugs) → feedback+
Comment on attachment 8944419 [details] [diff] [review] 2 - Bug 1379466 - Use auxclick event to trigger new tab on middle click. We'll need some test for auxclick handling here.
Attachment #8944419 - Flags: feedback?(bugs) → feedback+
"* some chrome code will start getting non-primary clicks where it didn't before." is worrisome
Comment on attachment 8944420 [details] [diff] [review] 3 - Bug 1379466 - Stop dispatching click events for non-primary mouse clicks on the web. This feels a bit risky. Could you clarify why you ended up going this route?
Attachment #8944420 - Flags: feedback?(bugs) → feedback-
Comment on attachment 8944423 [details] [diff] [review] 5 - Bug 1379466 - Remove dead middle click dispatch code from HTML(Input|TextArea)Element.cpp. (mNoContentDispatch is such a horrible hack I added 2006 to keep the old behavior. Good to see it being used less.) Why we can't remove if (aVisitor.mEvent->mFlags.mNoContentDispatch) { aVisitor.mItemFlags |= NS_NO_CONTENT_DISPATCH; } ?
Attachment #8944423 - Flags: feedback?(bugs) → feedback+
Comment on attachment 8944422 [details] [diff] [review] 4 - Bug 1379466 - Make editor listen for auxclick mouse events. Perhaps, you also need to add auxclick event listeners in HTMLEditor::AddMouseClickListener() and remove them from HTMLEditor::RemoveMouseClickListener() but looks fine for the others.
Attachment #8944422 - Flags: feedback?(masayuki) → feedback+
(In reply to Olli Pettay [:smaug] from comment #30) > Comment on attachment 8944419 [details] [diff] [review] > 2 - Bug 1379466 - Use auxclick event to trigger new tab on middle click. > > We'll need some test for auxclick handling here. I originally though the current tests might be adequate: https://searchfox.org/mozilla-central/rev/b7e3ec2468d42fa59d86c03ec7afeb209813f1d4/browser/base/content/test/general/browser_contentAreaClick.js#120-142 but looking further, while it does test that the new tab opening is attempted on middle click, it listens explicitly for the click event (which fires first so that's probably racy), and would fail if it was ever stopped fully dispatching. I can update that. (In reply to Olli Pettay [:smaug] from comment #32) > Comment on attachment 8944420 [details] [diff] [review] > 3 - Bug 1379466 - Stop dispatching click events for non-primary mouse clicks > on the web. > > This feels a bit risky. Could you clarify why you ended up going this route? Because once middle-click-open (and middle-paste) is done with auxclick instead of click, there's not much point dispatching clicks for them anymore, preventDefault() on them will have no effect, and the unnecessary events are likely to cause confusion and unwanted effects for web devs (as auxclick does now). Plus it seemed like it'd enable getting rid of some or all of the m(No|Force)ContentDispatch code. Couldn't get rid of them entirely since frontend code relies on non-primary clicks too much currently. I did discuss moving frontend to auxclick with Gijs but he thought it wouldn't be worth the hassle unless it lead to simplified code. Perhaps getting rid of this event special casing would count. (It also seemed like a long-winded project that could block this for a while) However, I was aware that this might be deemed too risky, so feel free to veto it. Just Ben's patch would be enough to clear the popup blocked message at least, though there'd still be the issue of two responses if there are both click and auxclick handlers. (In reply to Olli Pettay [:smaug] from comment #33) > Comment on attachment 8944423 [details] [diff] [review] > 5 - Bug 1379466 - Remove dead middle click dispatch code from > HTML(Input|TextArea)Element.cpp. > > (mNoContentDispatch is such a horrible hack I added 2006 to keep the old > behavior. Good to see it being used less.) > > Why we can't remove > if (aVisitor.mEvent->mFlags.mNoContentDispatch) { > aVisitor.mItemFlags |= NS_NO_CONTENT_DISPATCH; > } > ? You're right, we can. I did end up removing it later anyway in my patch for bug 329119 (Early version: https://hg.mozilla.org/try/rev/743bf4caa9ef9aeb63b940efe8eb61cfda26d0d1#l7.2). I think I didn't do it at the time because I just wasn't grokking the code well enough. (In reply to Olli Pettay [:smaug] from comment #31) > "* some chrome code will start getting non-primary clicks where it didn't > before." is worrisome So I originally though this was okay because I thought it only applied to the form autofill dialogs in about:preferences, which are special because they are (X)HTML documents with system principal and I thought rare, but I've now realised that that applies to a bunch of the about: pages as well (I thought more were XUL), not to mention the devtools that are already full HTML, so dropping mNoContentDispatch seems like a no go for now. Will have to use both it and mOnlyChromeDispatch as well. However an alternative approach (or follow up for bug 329119) would be to make use of a new event flag, something like mOnlyXulDispatch, that means those events only fire on targets in XULDocuments. While we could keep the not-dispatched to HTML elements behaviour in XULDocuments by keeping the ForceContentDispatch flag (or renamed equiv.) around, and moving it from nsDocument to XULDocument, I think we could just dispatch them to HTML elements in XUL, and rip out all the ForceContentDispatch stuff. I don't think there'd be much affected (the Exit Full Screen button in DOM Fullscreen is one, and some devtools code), and it's straightforward to fix by putting in a if (event.button != 0) return; check. (and some of the devtools is already inconsistent, XUL elements inappropriately reacting to middle/right click, HTML not just because they don't get it)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #34) > Comment on attachment 8944422 [details] [diff] [review] > 4 - Bug 1379466 - Make editor listen for auxclick mouse events. > > Perhaps, you also need to add auxclick event listeners in > HTMLEditor::AddMouseClickListener() and remove them from > HTMLEditor::RemoveMouseClickListener() but looks fine for the others. Thanks for the heads up about this! I've had a look, and I don't think it's necessary though. As far as I can tell, those are only used on the table editing row and column controls, and they would never have received non-primary clicks since they are all <a> elements, so they don't do the punching through mNoContentDispatch that input and textarea do, and the listeners aren't system listeners.
I noticed what seemed like an omission from the event synthesis helper code.
Attachment #8944422 - Attachment is obsolete: true
Attachment #8944423 - Attachment is obsolete: true
Attachment #8947894 - Attachment description: 5 - Bug 137944 - Stop dispatching click events for non primary mouse clicks on the web → 4 - Bug 137944 - Stop dispatching click events for non primary mouse clicks on the web
kwan: What's the status of this bug? I realized that a lot of default handler listens "click" rather than "auxclick" causes blocking other web-compat fix. Do you have a plan to fix this soon?
Ah, perhaps, event order should be discussed in bug 1441755. Sorry for the spam.
See Also: → 1441755
Flags: needinfo?(bugs)
QA Contact: overholt

Enable synthesising explicit auxclicks, and check that synthesised and
non-synthesised auxclicks get isHandlingUserInput false and true respectively.

So it is still preventDefault()able once non-primary clicks aren't web visible.

Editable elements will no longer get click events for non-primary mouse buttons
since they are being unshipped from the web in favour of auxclick events.
Listen for auxclick as well so middle-click paste still works.
Don't stop propagation after middle-click paste, instead ignore clicks on
contenteditable elements in ClickHandlerChild.
Update test_middle_click_paste.html for the new behaviour.

Also remove the mNoContentDispatch overrides in HTMLInputElement and
HTMLTextAreaElement that were needed for middle-pasting.

It's against spec, and the auxclick event is now available and easier to use
(fired on all elements rather than just window/document and text fields)
Can't stop dispatching them entirely since frontend code is too reliant on it.

Don't fire dblclick for auxclick.
Mark wpt uievents/click/auxclick_event.html as passing
Update test_clipboard_events.html

So this seems to be causing more web compat problems now, e.g. instagram seems to implement its own middleclick handling for links (god knows why) which doesn't work in Fx atm because of this, and sheppy reported problems with zenhub

This currently has more behaviour changes than before though, since I also (think I) fixed middlemouse paste handling so it can be cancelled and doesn't need to stopPropagation.

Probably needs more tests.

Seems reasonably green:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d10c2ecb8a50ef1a84cbc13e206d60e370ff74d

ok, this is now being fixed in other bug too...
I'll look at the patches tomorrow.

Attachment #8944418 - Attachment is obsolete: true
Attachment #8947891 - Attachment is obsolete: true
Attachment #8947892 - Attachment is obsolete: true
Attachment #8947893 - Attachment is obsolete: true
Attachment #8947894 - Attachment is obsolete: true

If needed for web-compat.
Also stop dispatching auxclicks if non-primary click has been preventDefaulted,
so that legacy new-tab prevention can work with the pref flip.

Thanks a lot smaug and masayuki!

Sheriffs, green try here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f967c0c8be6cff65079f7b16a0188958db6aa68

I have an older one with macosx and Android as well here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=36f06b268a3cc11cd0a9f624b7ff646506c9952b

which had a small bustage on linux debug M(cl) that was subsequently fixed.

Assignee: nobody → moz-ian
Status: NEW → ASSIGNED
Keywords: checkin-needed

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32d8c9a17e50
Add auxclick to mouse synthesis test code. r=smaug
https://hg.mozilla.org/integration/autoland/rev/8ff0d835d90b
Set EventPopupControlState of auxclick as 'openControlled'. r=smaug
https://hg.mozilla.org/integration/autoland/rev/4e152cfac89d
Use auxclick event to trigger new tab on middle click. r=smaug
https://hg.mozilla.org/integration/autoland/rev/273553e141f1
Make editor listen for auxclick mouse events. r=smaug,masayuki
https://hg.mozilla.org/integration/autoland/rev/0e9fa06f3fd8
Stop dispatching click events for non-primary mouse clicks on the web. r=smaug
https://hg.mozilla.org/integration/autoland/rev/23ff9cd1a1a4
Add override pref to restore legacy non-primary click dispatch on specific domains. r=smaug

Keywords: checkin-needed

Some rough notes for what needs documenting:

Firefox no longer dispatches to web content "click" events for buttons other than the left mouse button, following spec. Previously it would dispatch them just on the window global and the document object, and in <input> and <textarea> elements, but not on any other elements in the DOM tree. (So if you middle clicked on an <a> inside a <body>, neither the <a>, nor the <body>, nor the <html> would see the event, but the new tab could be blocked with a listener on the document or window).

Accordingly, new tab from middle click is now opened on the "auxclick" event (which is dispatched to the entire DOM tree), so developers need to use an "auxclick" instead of "click" event listener if they want to prevent new tabs. Developers are now also able to open new tabs/windows from "auxclick" event handlers (previously popup blocker blocked them).

The same is true for the middle-mouse paste feature (enabled by default on Linux, preffed-off on Windows and maxOS but I believe works when preffed-on). Middle mouse paste was the reason the editable <input> and <textarea> elements had the "click" carve-out, but it too now happens on the "auxclick" event. (middle mouse pasting also used to "eat"/stopPropagation of the "click" when pasting, so it couldn't be detected in bubbling, only capturing. "auxclick" pasting does bubble)

Per-spec, "dblclick" events are no longer dispatched for buttons other than the left button (it used to be dispatched to the same limited points of window/document/<input>/<textarea> for middle/right buttons).

Another behaviour change: middle-clicking a link in a designMode=on doc/contenteditable would open the link if middlemouse paste was off, otherwise it would paste text inside the link. Now it never opens the link (use right-click + context menu to do so).

I believe those are all the differences.

Keywords: dev-doc-needed

Any changes to "paste as quotation", even in FF, Ctrl+middle-mouse-click, with pref middlemouse.paste set to true?

(In reply to Jorg K (GMT+2) from comment #57)

Any changes to "paste as quotation", even in FF, Ctrl+middle-mouse-click, with pref middlemouse.paste set to true?

Ah, thank you for asking again, I forgot about checking that. That has had the same changes as regular middle-mouse paste, also moving to "auxclick", and the event bubbles.

OK, but does it still work in FF? It's used to exercise some of the "paste as quotation" editor functions in FF.

Yes, I was testing in Firefox (assuming the expected result is "paste with '> ' prepended" in <textarea>, and "as <blockquote> element" in contentEditable).

Yep, that's it.

Depends on: 1556658
Regressions: 1556658

Documentation changes:

Keywords: dev-doc-needed → dev-doc-complete
No longer depends on: 1556658
No longer regressions: 1556658
Regressions: 1878196
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: