[Webkit-unassigned] [Bug 53296] Implicit form submission shouldn’t work when the default button is disabled

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53296





--- Comment #9 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2011-01-31 10:06:02 PST ---
(From update of attachment 80632)
View in context: https://bugs.webkit.org/attachment.cgi?id=80632&action=review

>>> Source/WebCore/html/HTMLFormElement.cpp:188
>>> +        if (formElement->getAttribute(typeAttr).lower() == "submit" || formElement->hasLocalName(buttonTag)) {
>> 
>> This is horrible. We shouldn't need to reach into attributes at this level. Why did you change this?
> 
> I didn’t change this — it’s new code.
> Why did I write it like this? I simply don’t know of a better way. There’s no `isSubmitButton()` method. Should I add one? Any suggestions?

Yes -- you can plumb HTMLInputElement::isSubmitButton() and remove the use of isSuccessfulSubmitButton altogether. Just check for disabled().

Also, you don't need two variables here. Increment when found, decrement if disabled. You only need to make sure that there's at least one non-disabled submit button.

>>> LayoutTests/fast/forms/implicit-submission.html:-19
>>> -    [ "Text input and text area and a disabled submit, text input focused", "!text,textarea,-submit", "y" ], // match IE, not FF.
>> 
>> Aha. This is interesting. So this change may break sites, expecting IE behavior?
> 
> The spec states that if the only submit button in a form is disabled, it shouldn’t submit implicitly. If there are no submit buttons at all, implicit submission should still work. This patch would make it so.
> 
> So yes, it would break sites that rely on IE-style behavior for forms that only have disabled submit buttons. (Not that it matters, but that sure sounds like an edge case to me.) For this particular case, it would instead match Firefox and Opera’s behavior, which happens to conform to the HTML spec and also makes much more sense.
> 
> Would this change be acceptable or not?

I think the change is acceptable, but we should watch for the reactions from the Web and be ready to revert this if it proves drastic.

Also, would love to see a test added with multiple submit buttons (some disabled), just to test your newly added code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.