event.target refers to parent BUTTON instead of clicked IMG when IMG is child of BUTTON (Selecting different views of the product is not possible in www.myntra.com)

NEW
Assigned to

Status

()

Core
DOM: Events
P2
normal
3 years ago
14 days ago

People

(Reporter: Alwyn Edison Mendonca, Assigned: smaug)

Tracking

(Blocks: 1 bug)

Trunk
Other
Android
Points:
---
Dependency tree / graph
Bug Flags:
webcompat ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [country-in] [js] [webcompat], URL)

Attachments

(4 attachments)

(Reporter)

Description

3 years ago
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.
Status: UNCONFIRMED → NEW
Component: General → Mobile
Ever confirmed: true
Product: Firefox for Android → Tech Evangelism
Version: Firefox 33 → Trunk
(Reporter)

Comment 2

3 years ago
Yes works fine on mobile chrome browser. Thanks.

Updated

3 years ago
See Also: → bug 974793
Whiteboard: [country-in]
(Reporter)

Comment 3

3 years ago
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.

Comment 4

3 years ago
Something related to function nrWrapper() ?
in the main page.
Flags: needinfo?(hsteen)
Whiteboard: [country-in] → [country-in] [js]
(Reporter)

Comment 5

3 years ago
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/
Component: Mobile → DOM: Events
Flags: needinfo?(hsteen)
Product: Tech Evangelism → Core
Summary: Selecting different views of the product is not possible in www.myntra.com → event.target refers to parent BUTTON instead of clicked IMG when IMG is child of BUTTON (Selecting different views of the product is not possible in www.myntra.com)
Created attachment 8539656 [details]
TC - 1089326.htm

I'm surprised if this isn't a known issue already. Boris?
Flags: needinfo?(bzbarsky)
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.
Flags: needinfo?(bzbarsky)
I can't find this stuff spec'ed anywhere. Anne, can you help?
Flags: needinfo?(annevk)
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 ;)

Comment 14

3 years ago
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?
Flags: needinfo?(annevk)
(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.

Comment 16

3 years ago
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 :)
Flags: needinfo?(ian)
(Reporter)

Comment 18

3 years ago
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 :)

Comment 22

3 years ago
>>  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?

Comment 24

3 years ago
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.

Comment 26

3 years ago
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.
Created attachment 8647470 [details]
1089326.htm with extra listener on IMG and more logging

Comment 28

3 years ago
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.

Comment 33

2 years ago
(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?

Comment 35

2 years ago
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?
Flags: needinfo?(bugs)
Flags: needinfo?(bugs)
QA Contact: bugs
Assignee: nobody → bugs
QA Contact: bugs
Created attachment 8772586 [details] [diff] [review]
v1 with tests

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>

Updated

a year ago
Whiteboard: [country-in] [js] → [country-in] [js] [webcompat]

Comment 43

11 months ago
Created attachment 8844338 [details]
button-test.html

Comment 44

11 months ago
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

Updated

9 months ago
Duplicate of this bug: 1276143

Updated

9 months ago
Duplicate of this bug: 1050625

Updated

5 months ago
Flags: webcompat?

Updated

4 months ago
See Also: → bug 1397981
Blocks: 1406505
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)
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.