User Agent: Mozilla/5.0 (Android; Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Build ID: 20141011014717 Steps to reproduce: I visited http://www.myntra.com/Tshirts/Superdry/Superdry-Men-Green--White-Grit-Warriors-T-shirt/420443/buy?serp=25&searchQuery=men%20tees%20polos this product and tried to see different viws, but nothing is happening when I select them. Actual results: After selecting a product I tried to see that product in different view, like side view, back view, closer view and front. Which is not working. Expected results: After selecting different view of the product it has to display them.

Reproducible on desktop as well with the mobile UA; work's fine in Chrome desktop/mobile.

Yes works fine on mobile chrome browser. Thanks.

Desktop view is totally different and mobile view what I am getting is different same in chrome too. But chrome browser does the required job for me.

Something related to function nrWrapper() ? in the main page.

Ok. Is it possible to fix this issue??

This seems a core issue, not a site issue. The problem is that they try to read the new src attribute for the big image from the thumbnail DOM element with this code in the event listener: toggle:function(e){ ... i=e.target.getAttribute('data-main'); but the target of this event isn't what they expect. The markup is <button><img data-main="..." ... ></button> and Gecko sets event.target to button, not img. Demo: http://jsfiddle.net/hallvors/tthfrow2/

I'm surprised if this isn't a known issue already. Boris?

This is, or at least was, expected behavior. (HTML4 sort of wanted this behavior). Gecko and IIRC Netscape too have always had this.

hm - interesting! Both old Presto-based Opera and the most recent IE agree with Gecko. So it seems to be blink/webkit against all others.

Buttons effectively capture events, quite purposefully, like Olli said.

I can't find this stuff spec'ed anywhere. Anne, can you help?

IIRC the semi-defined behavior got lost in the specs when moving from HTML4 to HTML. HTML4 has stuff like "Buttons created with the BUTTON element function just like buttons created with the INPUT element, but they offer richer rendering possibilities"

I'm asking because if it's in a spec or on its way into a spec, we'll report Blink/WebKit bugs on fixing it in those engines and contact the site to try to get them to solve it. (Shouldn't be that hard when it fails in several browsers). It does however seem a bit of an anomaly or exception from the usual event model, and this bug confirms it can be a gotcha to developers. Perhaps going the other way would simplify specs and implementations? WebKit-based browsers obviously haven't had enough compat pain to align, so presumably changing it won't hurt us much in terms of compat either. Simplifying things sounds good to me ;)

This is defined as part of https://html.spec.whatwg.org/#activation I think. E.g. in the "run authentic click activation steps" in step 4 it finds the nearest activatable element which would be the button in this scenario, no?

(In reply to Anne (:annevk) from comment #14) > This is defined as part of https://html.spec.whatwg.org/#activation I think. > E.g. in the "run authentic click activation steps" in step 4 it finds the > nearest activatable element which would be the button in this scenario, no? Well, we're on to something (thanks a lot - über-spec-explainer) - it will indeed find the button as the "nearest activatable element" (though oddly enough if the markup was <button><span><img></span></button> it seems that algorithm wouldn't - since it only looks at parent??). But I see nothing in those steps that says event.target should therefore be the button? It says "Dispatch event (the required click event) at target." - not at the found element e.

It looks recursively at the parent element. Perhaps Ian remembers why we are not adjusting the target of the event. Perhaps we decided at some point to not align with Gecko...

(In reply to Anne (:annevk) from comment #16) > It looks recursively at the parent element. Ah, I overlooked the "and return to the first step" part of step 2. > Perhaps Ian remembers why we are > not adjusting the target of the event. Perhaps we decided at some point to > not align with Gecko... Look forward to any comments from him :)

In amazon.in also almost similar kind of issue. I tried to zoom the product but my firefox 36.0.3 behaves very odd like it just flashes screen, sometimes it displays but when I tried to pinch zoom it then it shows menu to save images . Here is the link http://www.amazon.in/dp/B00OK2ZW5W#immersive-view_1426959078049

(In reply to Alwyn Edison Mendonca from comment #18) > In amazon.in also almost similar kind of issue. I tried to zoom the product > but my firefox 36.0.3 behaves very odd like it just flashes screen, > sometimes it displays but when I tried to pinch zoom it then it shows menu > to save images . This issue is different - the site doesn't allow pinch-to-zoom (probably for a good reason: they use a swipe-based effect to scroll through images, and pinch/swipe would become ambiguous unless the programmer was very careful. When you try to pinch-zoom, sometimes you pressed your finger on the screen until the browser shows the long-click menu. I hope this explains everything :)

I linked to this bug from https://code.google.com/p/chromium/issues/detail?id=418209

..now reported separately as https://code.google.com/p/chromium/issues/detail?id=519621

>> But I see nothing in those steps that says event.target should therefore be >> the button? It says "Dispatch event (the required click event) at target." >> - not at the found element e. > Perhaps Ian remembers why we are > not adjusting the target of the event. Perhaps we decided at some point to > not align with Gecko... It sounds to me like the conclusion on this point is that Gecko's behavior doesn't match the spec, but blink's does. Right? The spec and chromium's behavior of dispatching the event to the inner node (instead of the button like Gecko does) makes more sense to me. If the page has installed a click event handler on the inner node, shouldn't it get a chance to be invoked?

No, Firefox is correct: https://html.spec.whatwg.org/multipage/interaction.html#run-authentic-click-activation-steps

So I already said that and Hallvord pointed out it dispatches at /target/, not /e/. The reason it finds /e/ is to avoid activating multiple ancestor elements with activation behavior. So at this point I tend to agree with Rick's analysis, though I guess it would be good for bz and smaug to take another look at this.

The spec does indeed dispatch at /target/ so it seems Gecko's behaviour is wrong. Perhaps I misunderstood Anne earlier. The /e/ element in the spec is just used for the "pre-click activation" and "post-click activation" stuff. With Gecko's implementation, an event listener added to IMG in this test case will never fire.

The specification is generic for all elements with activation behavior. It seems like bz and smaug say there used to be some special magic for "buttons" (only <button>?) which disappeared. If no other browser has that I don't really see a good reason to introduce it.

Ok, I'll close the chromium bug and remove myself from this bug. Feel free to re-add me if any chromium-related questions/issues arise again.

What magical buttons? <button> is <button> and per HTML4 it was supposed to just use the contents of it as the UI. HTML spec doesn't actually define the behavior, since all it says is "Let target be the element designated by the user (the target of event)." In Gecko that target is <button>.

(In reply to Olli Pettay [:smaug] from comment #29) > In Gecko that target is <button>. ..and in Blink it might be a descendant of <button>, which is why we have this discussion. I think we need a spec bug to get this clarified. FWIW I find it weird that if I attach a click listener to a descendant of <button> the listener will never fire even if I attempt to click that element. <button><img onclick="..."></button> simply won't work in Gecko. Isn't that breaking some reasonable assumptions about how things work?

No. <button> is <button> (and <button type="button"> != <input type="button">) it is the event target and the contents of it just represent the UI. As HTML 4 says "Buttons created with the BUTTON element function just like buttons created with the INPUT element, but they offer richer rendering possibilities: the BUTTON element may have content."

That text from HTML 4 is not very precise (it doesn't really say anything about event handling or indicate that the authors considered what we're talking about) and therefore better considered obsoleted by the more detailed stuff in https://html.spec.whatwg.org/multipage/interaction.html#run-authentic-click-activation-steps . If we disagree with that, we should report a bug against HTML5 saying the algorithm should change to set target to the found "activation element" /e/ instead of /target/. As of now, HTML5 says Gecko is wrong and we can't use the handwavy HTML4 text to get away with it, IMHO.

(In reply to Hallvord R. M. Steen [:hallvors] from comment #32) > That text from HTML 4 is not very precise (it doesn't really say anything > about event handling or indicate that the authors considered what we're > talking about) and therefore better considered obsoleted by the more > detailed stuff in > https://html.spec.whatwg.org/multipage/interaction.html#run-authentic-click- > activation-steps . If we disagree with that, we should report a bug against > HTML5 saying the algorithm should change to set target to the found > "activation element" /e/ instead of /target/. As of now, HTML5 says Gecko is > wrong and we can't use the handwavy HTML4 text to get away with it, IMHO. I agree with this; and like to add if a decision needs to be made it needs to be made for the better of the web developer; and as in comment #30 the current Gecko behavior seems confusing. (Unless the developer consults the handy waving HTML4 documentation and interprets it to match what Gecko is doing).

How does <button> behave on Edge? IE is like Gecko, right?

Edge behaves like Chrome. IE behaves like Gecko.

Now we have another interesting case (on a pretty important website FWIW - outlook.com) in bug 1277620. Basically it boils down to <button onclick="..."> .. <button onclick="...">X</button></button> with the page expecting the inner BUTTON element's onclick to be triggered if you click/tap where the inner BUTTON is. In my testing, Chrome (Win8), Safari (Mac), IE11 (Win8) and Edge (Win10) all do what the site (and per my reading, the HTML spec) expect - consider the inner button the "nearest activatable element" per https://html.spec.whatwg.org/multipage/interaction.html#run-authentic-click-activation-steps and fire that click event on the inner BUTTON element. Gecko (and Opera Presto - RIP) do NOT fire click on the inner BUTTON. I recommend we change ;)

(In reply to Hallvord R. M. Steen [:hallvors] from comment #36) > > I recommend we change ;) Olli, WDYT?

aVisitor.mEvent->mOriginalTarget == this change is a bit controversial but follows the model we have for links http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/dom/base/Element.cpp#3039 And DOMActivate is really super legacy and deprecated. Let's see how many tests are broken https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b5045f276a45dfe9a31a8f0b155a6691d4e3cc9

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5f400f9815f with new click event targeting, following the model from blink (which is rather different from what blink devs have told me before!)

Something which seems slightly similar. https://webcompat.com/issues/3575 with a <button class="btn btn-blue"> <a href="..."></a> </button>

Because there is another case with: https://webcompat.com/issues/4909 I created a test case for <button> <a href="…"></a> </button> http://codepen.io/webcompat/pen/QpKPeR https://bugzilla.mozilla.org/attachment.cgi?id=8844338 # Link does NOT work * Firefox Nightly 54. # Link works * Opera Beta 44 / Chrome/57.0.2987.19 * Safari 10.0.3 (12602.4.8) To test: IE/Edge

Not sure if it also effects: https://webcompat.com/issues/13103

https://github.com/webcompat/web-bugs/issues/14354 seems like another example of this bug: <li> <button aria-label=foo> <div>foo</div> </button> </li> var a = event.target.parentNode.getAttribute('aria-label'); a is undefined in Firefox, and "foo" in Chrome (which the site depends on for menu navigation)

Would this bug also cause the tooltip for an <img> with a title to not be shown if it's wrapped in a <button>? E.g. <button> <img title="this title is not shown on mouse hover" /> </button>

Yes. Internal's of <button> aren't part of hit testing.

remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=779907fb1934aeb1fdf81bc2fe671267d8fbb1f3 remote: recorded changegroup in replication log in 0.008s

remote: View your change here: remote: https://hg.mozilla.org/try/rev/b816ed05da791b533daa55e8f8eee3eaa2b174d5 remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b816ed05da791b533daa55e8f8eee3eaa2b174d5 remote: recorded changegroup in replication log in 0.006s

This changes click event handling to follow similar model what blink has, which is not the spec model. But blink might be ready to follow the spec. remote: View your change here: remote: https://hg.mozilla.org/try/rev/ea7977629d5b9a6515b6b1421741019338d256f2 remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea7977629d5b9a6515b6b1421741019338d256f2 remote: recorded changegroup in replication log in 0.045s

Some fixes to pass marionette tests.

remote: View your change here: remote: https://hg.mozilla.org/try/rev/14ab7f707574031c2342ebe119ec7640d226b9a5 remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14ab7f707574031c2342ebe119ec7640d226b9a5 remote: recorded changegroup in replication log in 0.019s

(FWIW, I'm actively fixing the tests and FF UI code this change is breaking)

I'll split this to smaller pieces and FF UI fixes to separate bugs.

ok, so this stuff really needs to go in as single push. I split some testing or FF UI related issues to separate bugs. There are two major pieces here. (1) Make button hit testing work like other elements which may have some child nodes. That is what nsHTMLButtonControlFrame.cpp is about and also the changes to HTMLButtonElement.cpp and HTMLInputElement.cpp to deal with nested submit buttons. (2) Change to click handling to follow blink model (which is quite close to the spec.) Fire click on the closest ancestor of mousedown/up targets, but "Interactive Content" makes this a tad trickier. See the comment in nsContentUtils.h. This change is mostly just in EventStateManager::SetClickCount and nsContentUtils::GetCommonAncestorUnderInteractiveContent Overall the changes are surprisingly small.

Comment on attachment 9016468 [details] [diff] [review] button_click_3_mod.diff Review of attachment 9016468 [details] [diff] [review]: ----------------------------------------------------------------- I hope that I don't fail finding some wrong changes in HTMLInputElement.cpp though (it's too big to review with Splinter). ::: dom/base/nsContentUtils.cpp @@ +2659,5 @@ > } > > +// static > +nsINode* > +nsContentUtils::GetCommonAncestorUnderInteractiveContent(nsINode* aNode1, I'm still not sure what is the standard, though. This method stops climbing up the tree when it meets an interactive element. That means that when 2 DOM points under same interactive element but at least one of them is in another interactive element, this does not return the ancestor interactive element. Is this intended? E.g., <a href="foo"> <button>button</button> anchor </a> mousedown in the <button> but mouseup in the text node after <button>, click event shouldn't be fired on the <a>. Is this intended behavior? As far as I've tested, Chrome does not fire click event in this case (with pressing Alt, but looks like that Chrome does not fire click event if selection is changed by the drag anyway). https://jsfiddle.net/d_toybox/mhga9xjd/1/ @@ +2696,5 @@ > + uint32_t pos1 = parents1.Length(); > + uint32_t pos2 = parents2.Length(); > + nsINode* parent = nullptr; > + uint32_t len; > + for (len = std::min(pos1, pos2); len > 0; --len) { Why don't you define |len| in the condition of this for loop? I mean, |for (uint32_t len = ...)| ::: dom/events/EventStateManager.cpp @@ +4898,4 @@ > } else if (aEvent->mMessage == eMouseUp) { > + aEvent->mClickTarget = > + nsContentUtils::GetCommonAncestorUnderInteractiveContent( > + mouseContent, mLastLeftMouseDownContent); Although I believe that this is the reasonable way for realistic cases. However, if mLastLeftMouseDownContent has been moved to different parent and it *now* has a common interactive content with mouseup content, this fires click event (i.e., even if mousedown content was outside of the interactive content). Is it okay? I guess okay, but perhaps, not declared in any standards. @@ +4960,5 @@ > return false; > } > // If mouse is still over same element, clickcount will be > 1. > // If it has moved it will be zero, so no click. > + if (!aMouseEvent.mClickCount || !aMouseEvent.mClickTarget) { Ah, sorry for you rewriting this patch due to my patch. ::: dom/events/test/test_dblclick_explicit_original_target.html @@ +22,5 @@ > synthesizeMouse(document.getElementById("display"), 5, 5, { clickCount: 2 }); > } > > +window.onmousedown = function(event) { > + is(event.explicitOriginalTarget.nodeType, Node.TEXT_NODE, "explicitOriginalTarget is a text node"); Why don't you compare event.explicitOriginalTarget and document.getElementById("display").firstChild directly? @@ +26,5 @@ > + is(event.explicitOriginalTarget.nodeType, Node.TEXT_NODE, "explicitOriginalTarget is a text node"); > +} > + > +window.onmouseup = function(event) { > + is(event.explicitOriginalTarget.nodeType, Node.TEXT_NODE, "explicitOriginalTarget is a text node"); Ditto. @@ +35,2 @@ > window.ondblclick = function(event) { > + is(event.explicitOriginalTarget.nodeType, Node.ELEMENT_NODE, "explicitOriginalTarget is an element node"); Similar to the above, event.explicitOriginalTarget should be document.getElementById("display") in this case? ::: dom/html/HTMLInputElement.cpp @@ +138,4 @@ > #define NS_CONTROL_TYPE(bits) ((bits) & ~( \ > NS_OUTER_ACTIVATE_EVENT | NS_ORIGINAL_CHECKED_VALUE | NS_NO_CONTENT_DISPATCH | \ > + NS_ORIGINAL_INDETERMINATE_VALUE | NS_PRE_HANDLE_BLUR_EVENT | \ > + NS_PRE_HANDLE_INPUT_EVENT | NS_IN_SUBMIT_CLICK)) Are you fixing different bug to include NS_PRE_HANDLE_BLUR_EVENT and NS_PRE_HANDLE_INPUT_EVENT into NS_CONTROL_TYPE? I don't find any reason to fix this now except you are adding (aVisitor.mItemFlags & NS_IN_SUBMIT_CLICK) check below. I'd be clearer if you fix such kind of change is separated to a preceding patch or mentioned in commit message. ::: dom/html/test/test_bug1089326.html @@ +17,5 @@ > + var b_rect = b.getBoundingClientRect(); > + var a = document.getElementById("anchor"); > + var a_rect = a.getBoundingClientRect(); > + > + is(document.elementFromPoint(b_rect.x + 1, b_rect.y + 1), b); Please add message of this is() like "<button> element should be at {1-1} in its rect" or something. @@ +18,5 @@ > + var a = document.getElementById("anchor"); > + var a_rect = a.getBoundingClientRect(); > + > + is(document.elementFromPoint(b_rect.x + 1, b_rect.y + 1), b); > + is(document.elementFromPoint(a_rect.x + 1, a_rect.y + 1), a); Ditto. @@ +54,5 @@ > + var span2 = document.getElementById("span2"); > + expectedTarget = container; > + synthesizeMouseAtCenter(span1, { type: "mousedown" }); > + synthesizeMouseAtCenter(span2, { type: "mouseup" }); > + is(clickCount, 2, "Should not have got a click event."); s/not// @@ +56,5 @@ > + synthesizeMouseAtCenter(span1, { type: "mousedown" }); > + synthesizeMouseAtCenter(span2, { type: "mouseup" }); > + is(clickCount, 2, "Should not have got a click event."); > + > + SimpleTest.finish(); Oh, you don't test synthesizeMouseAtCenter() with button#button and a#anchor. Is it intentional? Or just forgot? ::: widget/MouseEvents.h @@ +303,5 @@ > } > > + // If during mouseup handling we detect that click event might need to be > + // dispatched, this is setup to be the target of the click event. > + nsCOMPtr<dom::EventTarget> mClickTarget; Okay, looks like that this is not necessary to copy with Duplicate() nor AssignMouseEventData().

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #68) >I hope that I don't fail finding some wrong changes in HTMLInputElement.cpp though (it's too big to >review with Splinter). Sorry about that. I don't ever use splinter and prefer plain raw bugzilla patches for reviewing. > > +// static > > +nsINode* > > +nsContentUtils::GetCommonAncestorUnderInteractiveContent(nsINode* aNode1, > > I'm still not sure what is the standard, though. > Yeah, I'm following blink model here. > This method stops climbing up the tree when it meets an interactive element. > That means that when 2 DOM points under same interactive element but at > least one of them is in another interactive element, this does not return > the ancestor interactive element. Is this intended? E.g., yes > > <a href="foo"> > <button>button</button> > anchor > </a> > > mousedown in the <button> but mouseup in the text node after <button>, click > event shouldn't be fired on the <a>. Is this intended behavior? yup. At least for now. We may be able to remove the interactive content handling. > > @@ +2696,5 @@ > > + uint32_t pos1 = parents1.Length(); > > + uint32_t pos2 = parents2.Length(); > > + nsINode* parent = nullptr; > > + uint32_t len; > > + for (len = std::min(pos1, pos2); len > 0; --len) { > > Why don't you define |len| in the condition of this for loop? > I mean, |for (uint32_t len = ...)| looks like a copy-paste from other similar method > > ::: dom/events/EventStateManager.cpp > @@ +4898,4 @@ > > } else if (aEvent->mMessage == eMouseUp) { > > + aEvent->mClickTarget = > > + nsContentUtils::GetCommonAncestorUnderInteractiveContent( > > + mouseContent, mLastLeftMouseDownContent); > > Although I believe that this is the reasonable way for realistic cases. > > However, if mLastLeftMouseDownContent has been moved to different parent and > it *now* has a common interactive content with mouseup content, this fires > click event (i.e., even if mousedown content was outside of the interactive > content). Is it okay? I guess okay, but perhaps, not declared in any > standards. interactive content part isn't defined, but even if we take that out, mousedown/up should fire click when under the same subtree. though, moving mousedown target is indeed unclear. Something to fix in UI Events spec. > > + if (!aMouseEvent.mClickCount || !aMouseEvent.mClickTarget) { > > Ah, sorry for you rewriting this patch due to my patch. no problem. Rebasing was easy. > > ::: dom/events/test/test_dblclick_explicit_original_target.html > @@ +22,5 @@ > > synthesizeMouse(document.getElementById("display"), 5, 5, { clickCount: 2 }); > > } > > > > +window.onmousedown = function(event) { > > + is(event.explicitOriginalTarget.nodeType, Node.TEXT_NODE, "explicitOriginalTarget is a text node"); > > Why don't you compare event.explicitOriginalTarget and > document.getElementById("display").firstChild directly? Just being consistent with the existing test. > @@ +35,2 @@ > > window.ondblclick = function(event) { > > + is(event.explicitOriginalTarget.nodeType, Node.ELEMENT_NODE, "explicitOriginalTarget is an element node"); > > Similar to the above, event.explicitOriginalTarget should be > document.getElementById("display") in this case? Yeah, I can change. > ::: dom/html/HTMLInputElement.cpp > @@ +138,4 @@ > > #define NS_CONTROL_TYPE(bits) ((bits) & ~( \ > > NS_OUTER_ACTIVATE_EVENT | NS_ORIGINAL_CHECKED_VALUE | NS_NO_CONTENT_DISPATCH | \ > > + NS_ORIGINAL_INDETERMINATE_VALUE | NS_PRE_HANDLE_BLUR_EVENT | \ > > + NS_PRE_HANDLE_INPUT_EVENT | NS_IN_SUBMIT_CLICK)) > > Are you fixing different bug to include NS_PRE_HANDLE_BLUR_EVENT and > NS_PRE_HANDLE_INPUT_EVENT into NS_CONTROL_TYPE? yeah, fixing NS_CONTROL_TYPE to not include those flags I don't find any reason to > fix this now except you are adding (aVisitor.mItemFlags & > NS_IN_SUBMIT_CLICK) check below. I'd be clearer if you fix such kind of > change is separated to a preceding patch or mentioned in commit message. True, it would have been better. > ::: dom/html/test/test_bug1089326.html > @@ +17,5 @@ > > + var b_rect = b.getBoundingClientRect(); > > + var a = document.getElementById("anchor"); > > + var a_rect = a.getBoundingClientRect(); > > + > > + is(document.elementFromPoint(b_rect.x + 1, b_rect.y + 1), b); > > Please add message of this is() like "<button> element should be at {1-1} in > its rect" or something. ok > @@ +56,5 @@ > > + synthesizeMouseAtCenter(span1, { type: "mousedown" }); > > + synthesizeMouseAtCenter(span2, { type: "mouseup" }); > > + is(clickCount, 2, "Should not have got a click event."); > > + > > + SimpleTest.finish(); > > Oh, you don't test synthesizeMouseAtCenter() with button#button and > a#anchor. Is it intentional? Or just forgot? I can add another test > > > > + // If during mouseup handling we detect that click event might need to be > > + // dispatched, this is setup to be the target of the click event. > > + nsCOMPtr<dom::EventTarget> mClickTarget; > > Okay, looks like that this is not necessary to copy with Duplicate() nor > AssignMouseEventData(). yeah, mClickTarget needs to keep the target only temporarily.

I came across a similar issue Please refer https://codepen.io/sarbbottam/pen/vVJQjy <button> <span>Click Me!</span> </button> e.target & e.currentTarget both are BUTTON in firefox. But in Chrome, Edge, Safari, Opera e.target is SPAN & e.currentTarget is BUTTON. For screenshots please refer https://twitter.com/sarbbottam/status/1050984396273082368

Is the lack of interactivity of data:text/html, <button><input type=range></button> this same issue?

Yes. And I'm working on this actively. Windows TVw test just doesn't like me.

rebased

Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/36b949bef798 make <button> hit testing similar to other elements which may have some content, and for click target find the common (interactive) ancestor, r=masayuki

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14478 for changes under testing/web-platform/tests

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Upstream PR merged

If this fix is proven to work in the 66 target I'd like to raise it for possible uplifting to 65 (and possibly 60esr). It seems to affect enough sites and there might be many more hidden.

This is massive behavioral change, scariest change I've done for years (changes how Gecko decides when to dispatch click and where), so we need to be quite sure this doesn't cause major regressions.

(In reply to Olli Pettay [:smaug] (high review load) from comment #83) > This is massive behavioral change, scariest change I've done for years > (changes how Gecko decides when to dispatch click and where), so we need to > be quite sure this doesn't cause major regressions. Agree we should ride the trains here.