65747 - [FIX][MF][IMG]<input type="image"> works only if the image can be loaded

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Stephan Niemz]

I've seen this for quite a while now, both on Linux and on NT4. Within a <form>,
<input type="image"> works only if the image can be loaded actually. If it can't
(because of a 404 or something), then you can't submit the form.

To Reproduce:

Open the attached test case with Mozilla. It contains a <form> with two
<imput type="image">s. The first one (Mozilla banner) is there and can be
clicked on. The second one can't be loaded, and so the alt-text is shown.
The cursor changes to a hand when moving over it, indicating that it is
clickable, but nothing happens if you click. Expected: submit the form.

Comment 1

[Stephan Niemz]

Attachment: test case

Comment 2

[Stephan Niemz]

Adding keywords:
4xp         since this works with NS 4.x
correctness
dataloss    because I have to switch to another browser and type in the whole
            form again
testcase

Comment 3

[kmike]

yep, <input type="image"> also do not work when image loading is disabled or
that particular image has been blocked. i think it is related to bug 41924, if
not dupe.

Comment 4

[Stephan Niemz]

I found bug 41924, too, when looking before filing this bug. But that bug has
many more issues open, this one is much more specific. Marking this bug as (yet
another) dependency of bug 41924, which is the IMHO better than dup'ing it and
then it gets lost.

And by the way, the bug I'm describing here isn't even mentioned in bug 41924.

Comment 5

[hirata masakazu]

Sounds like there is something common with the notorious PowerPC-only bug 63846.

Comment 6

[hirata masakazu]

The bug 63846 is fixed now, but this is not working.

Comment 7

[rods (gone)]

Will have to take a look

Comment 8

[hirata masakazu]

nominating for mozilla 0.8, since today is the last day, and this one definitely
deserves a relnote.

Comment 9

[Blake Ross]

Drivers determine .8 criticality.

Comment 10

[Kevin McCluskey (gone)]

Setting target to mozilla0.9.  

Comment 11

[rods (gone)]

Attachment: patch file

Comment 12

[rods (gone)]

Attachment: new patch with factored code limited to the frames

Comment 13

[Eric Pollmann]

Is there still a diff in nsHTMLInputElement::HandleDOMEvent with the new patch
to fix this bug?  All of the changes in nsHTMLInputElement.cpp in the first
patch looked fine, except the LEFT_BUTTON_UP should probably be
LEFT_BUTTON_CLICK so that if the button is depressed on another control and
dragged into the image input before being released, a submit will not be triggered.

r=pollmann for all of the changes in the new patch.  These changes are a nice
refactoring and make me think that we should probably move the OnSubmit() and
OnReset() calls in DoManualSubmitOrReset() to
nsHTMLFormElement::HandleDOMEvent() at some point so they parallel how all other
form events are processed - but that can wait...  :)

Comment 14

[rods (gone)]

No, there will be no changes to nsHTMLInputElement until we can factor that 
also.

Comment 15

[Marc Attinasi]

sr=attinasi

Comment 16

[rods (gone)]

Attachment: a different testcase

Comment 17

[rods (gone)]

For whatever reason the first testcase still work with my fix because the <p> 
tags are causing problems when inside the form, this is certainly a different 
bug, and I don't know if it is a know bug.

The orginal patch I attached had a lot of code factoring and it shared it 
between the frames lib and the content lib. Now that the content lib has been 
broken out on it's own I had to redo both parts of my fix. attinasi sr'ed the 
first part, the factoring of the code in the frames sections.

This next patch is more factoring and the actual fix. Both patch do contain the 
same method implmemntation which in the future we need to figure out how to put 
the code in a single spot so they both can call it. 

Comment 18

[rods (gone)]

Attachment: second half of the patch

Comment 19

[rods (gone)]

Boy do I need to review what I type more carefull, that comment is terrible, and 
I meant to say that the the original testcase still "doesn't" work, but like I 
said that is a different bug.

The only method that is duplicated in nsHTMLInputElement and nsFormControlHelper 
is "DoManualSubmitOrReset" myself or pollmann will open a bug to track that we 
merge those two impls.

Comment 20

[Eric Pollmann]

In the second patch, shouldn't NS_MOUSE_LEFT_BUTTON_UP be
NS_MOUSE_LEFT_BUTTON_CLICK to make the behaviour consistent with when the image
can be loaded?

Otherwise r=pollmann - this is refactoring that needed to be done, great!  :)
(I have a few ideas on how to combine the content and frames
DoManualSubmitOrReset methods - later work)

Comment 21

[Kevin McCluskey (gone)]

Set milestone to mozilla0.8.1

Comment 22

[rods (gone)]

fixed

Comment 23

[rods (gone)]

try this again - fixed

Comment 24

[Vladimir Ermakov]

Testcase still doesnt work for me on build 2001-03-08-04-Mtest, reopening

Comment 25

[rods (gone)]

My testcase dated "2001-02-22 17:59" works fine and I am marking this bug fixed 
once again. The original testcase still does not work because of a different 
bug. My understanding of the intent of this bug was that the alt text frames 
were not properly processing the event for the submit, which is what I fixed.

I have opened Bug 71476 for the problem with the <p> tag which is a core layout 
issue.

Comment 26

[rods (gone)]

remarking as fixed

Comment 27

[Vladimir Ermakov]

Ok, the second testcase works, verifying fixed.