282266 - <input type="image"> with DOM-dispatched click event does not compute x, y coordinates

Status: VERIFIED FIXED

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

Description

[Alex Vincent [:WeirdAl]]

If you manually click on an <input type="image"> element, Mozilla submits the
(x,y) coordinates, relative to the image's upper left corner, of where you clicked.

However, if a DOM HTML click event is dispatched to the same element to the same
screen coordinates, the submitted (x,y) pair is (0,0).

Testcase coming up in a few minutes.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: testcase, part 1 or 2

Comment 2

[Alex Vincent [:WeirdAl]]

Attachment: testcase, part 2 of 2

Steps to reproduce:
(1) Open the testcase.
(2) Click on the red box.
(3) Hixie's echo test tool will appear.  Note the values at the very end of the
page (approximately test.x=417&test.y=35).
(4) Go back to the testcase.
(5) Click on the button.
(6) Examine the echo test tool results again.

Expected results: 
test.x and test.y have specific values which are non-zero at the end of Hixie's
echo tool.

Actual results:
test.x=0&test.y=0 at the end of Hixie's echo tool.

Comment 3

[Alex Vincent [:WeirdAl]]

Here's what I think is happening:

nsHTMLFormElement.cpp#1062
<form> tells <input type="image"/> to report its controls.

nsHTMLInputElement.cpp#2179
<input type="image"/> looks up its X and Y coordinates from the image control frame.

nsImageControlFrame.cpp#375
This looks up the X coordinate from an internal value, mLastClickPoint.x.

Now, here's the catch:
nsImageControlFrame.cpp#140
The coordinates are initialized to (0,0)...

nsImageControlFrame.cpp#263
This line would have reset the coordinate appropriately, if the event was a
mouseup (line 259).

Since it's not, the click forces a submit of coordinates (0,0).

Am I close?  Suggestions?

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: testcase, part 2 of 2

Added an extra test per timeless:  we don't want to change the coordinates sent
by calling HTMLInputElement.click().

Comment 5

[Hixie (not reading bugmail)]

seems like using the click event instead of the mouseup event makes sense.

Comment 6

[Alex Vincent [:WeirdAl]]

Something rather interesting:  it doesn't matter if you forcibly dispatch a
click event or a mouseup event.  We never compute the coordinates.

Comment 7

[Alex Vincent [:WeirdAl]]

Attachment: patch, draft 1

This patch checks in nsHTMLInputElement to see if aDOMEvent is non-null.  I'm
not sure this is the right way to go (I'd prefer checking a property of aEvent,
instead).

Also, timeless has raised an issue for me to test with regard to image maps.  I
don't quite see how that could affect anything, but I will test.

Comment 8

[Alex Vincent [:WeirdAl]]

bz gave a sr- over irc, with two testcases.  Basically, custom positioning
changes the view that this code relies on.

Comment 9

[timeless]

jst: we'd like to get this into cvs, as is (with iid bumping as necessary), or
ifdef CENZIC if absolutely necessary. bz would prefer not ifdef'ing CENZIC.

Comment 10

[timeless]

Comment on attachment 174736 [details] [diff] [review]
patch, draft 1

this code is from Cenzic Hailstorm 2.5-2.6

Comment 11

[Boris Zbarsky [:bzbarsky]]

Er.. I'd prefer not ifdefing CENZIC for patches which are sort of correct.  This
change is not.

How about fixing this so that it uses client or screen throughout?  That is,
pass in the DOM event, get its screen coords, get the frame's screen coords,
etc.  That might work right in the cases this patch originally failed in...

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 174736 [details] [diff] [review]
patch, draft 1

Rev the IID in nsIImageControlFrame.

r+sr=jst

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 174736 [details] [diff] [review]
patch, draft 1

I'm actually going to mark this r- per comment 11.  Sending wrong data is worse
than sending no data at all.

Comment 14

[carglue a t ya hoo do tcom]

Some additional comments:
 
As mentioned, when submitting a form via an image input element, the POST 
string will be appended with 0,0 coords for the clicked input image element

...&MyImageName.x=0&MyImageName.y=0

Instead of appending the proper offset coords:

...&MyImageName.x=7&MyImageName.y=9

What may not be immediately apparent is that these extra post parameters let 
the server-side CGI scripts determine which image input element (out of a 
possible set on the page) was clicked. That is, the image name with non-zero x 
and y coords is the one that was clicked, all the others were not.

I too have confirmed that there is a difference between interactive and 
synthetic event creation.  Using an onSubmit event listener, I can see that 
when I click an image input element interactively, the event that is caught 
has an explicitOriginalTarget child node that points to the correct 
HTMLInputElement that I clicked on.  It shows the necessary image offset 
coordinates which are appended to the post string.  However, if I recreate the 
exact same event programmatically using initMouseEvent, that same onSubmit 
handler will show the explicitOriginalTarget as an HTMLFormElement elment 
instead, resulting in lost image offset data.
 
The only possible way I can see to around this is hacking the in-flight post 
data stream to override the .x=0 and .y=0 values to non-zero values so the 
client CGI can tell which image what selected -- a nasty workaround.  Is there 
a better alternative (workaround) that can be used in a Firefox extension that 
would not require all users to patch their Firefox client?

Comment 15

[Wladimir Palant]

I cannot reproduce this issue with Firefox 2.0.0.8. Alex, can you try again and resolve WORKSFORME if everything works?

Comment 16

[Alex Vincent [:WeirdAl]]

With Firefox 2.0.0.8 on WinXP, I get inconsistent results, depending on the order of clicks.

Comment 17

[carglue a t ya hoo do tcom]

Confirmed that this is still and issue in FF 2.0.0.16 and FF 3.0.1

I don't see any inconsistencies in the results I get, using different sites.
Here's an example of a POST that was generated after synthetically injecting mouse events (onMousedown, onMouseClick and onMouseUp):

"__EVENTTARGET=&__EVENTARGUMENT=&__VIEWSTATE=.....(middle content truncated).....&btnLogon.x=0&btnLogon.y=0"

Comment 18

[Colin Blake]

This is still an issue in FF3 (if a DOM HTML click event is dispatched the
submitted (x,y) pair is (0,0) regardless of what is passed in initMouseEvent).

The patch provided by Alex has rotted since the code it was playing with has
very much changed. Anyone know how to fix this in the scheme of things?

Comment 19

[Boris Zbarsky [:bzbarsky]]

I doubt that it's really changed that much, actually... should be a pretty straightforward port, in fact.

Comment 20

[Aaron Mansheim]

Status (three years later): reproducible in the versions that I have at hand.

Mozilla/5.0 (Windows NT 5.1; rv:5.0.1) Gecko/20100101 Firefox/5.0.1
http://hg.mozilla.org/releases/mozilla-release/rev/3ded311d93ad

Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20100101 Firefox/6.0
http://hg.mozilla.org/releases/mozilla-release/rev/218ed8178b1e

Comment 21

[Mike Hoye [:mhoye]]

Assuming this is still a thing, I'm going to recirculate this to the community.

Comment 22

[Kenneth Cochran [:kennycoc]]

As of the latest nightly, the provided test case returns either 0, 0; or if you have already manually clicked on the image, it will instead return the last clicked coordinates. I'd be willing to work on this, but may need some additional direction as this will be my first bug.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Kenneth, I'm happy to help out as needed.  Have you got a build environment set up and all that already?

Comment 24

[Kenneth Cochran [:kennycoc]]

I do have a build environment set up in Windows, and have been able to successfully build twice; however, it's a bit finicky, so I'm probably just going to download the build VM so I (hopefully) don't have to worry about it.

Comment 25

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 282266 - Move nsImageControlFrame declaration to header. r=#dom-core,#layout

In order to use nsImageControlFrame from HTMLInputElement without
including its implementation, nsImageControlFrame should have its own
header.

Comment 26

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 282266 - Populate imageClickedPoint when submitter is an image button. r=#dom-core,#layout

If the form submission originated from an event dispatch, this patch
ensures that the submitted image button coordinate is not the default.

Comment 27

[Zach Hoffman [:zrhoffman]]

https://treeherder.mozilla.org/jobs?repo=try&revision=af224231a3e66f94fb37d482725823255f02c951

Comment 28

[Zach Hoffman [:zrhoffman]]

Updated try run for D224651 Diff 924974: https://treeherder.mozilla.org/jobs?repo=try&revision=addfc7f14dcc836e028203607c8209aa2adbe5b3

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e37003ee9eec
Populate imageClickedPoint when submitter is an image button. r=dom-core,layout-reviewers,emilio,masayuki

Comment 30

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

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

Comment 31

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/e37003ee9eec

Comment 32

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 33

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2024-10-10 Nightly build on Windows 10.
Verified as fixed on Firefox Nightly 134.0a1 and Firefox 133.0 on Windows 10, Ubuntu 22, macOS 14.