[webkit-reviews] review denied: [Bug 53296]=?UTF-8?Q?=20Implicit=20form=20submission=20shouldn=E2=80=99t=20work=20when=20the=20default=20button=20is=20disabled=20?=: [Attachment 80632] Revised patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Mathias Bynens
<mathias at qiwi.be>'s request for review:
Bug 53296: Implicit form submission shouldn’t work when the default button is
disabled
https://bugs.webkit.org/show_bug.cgi?id=53296

Attachment 80632: Revised patch
https://bugs.webkit.org/attachment.cgi?id=80632&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80632&action=review

> Source/WebCore/ChangeLog:5
> +	   Implicit form submission shouldn’t work when the default button is
disabled https://bugs.webkit.org/show_bug.cgi?id=53296

What's that odd character in shouldn't? Bug URL should go on a different line.

> Source/WebCore/ChangeLog:6
> +

There is usually a mention of a test here. Like so:

Test: fast/forms/implicit-submission.html

> 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?

> LayoutTests/ChangeLog:5
> +	   Implicit form submission shouldn’t work when the default button is
disabled https://bugs.webkit.org/show_bug.cgi?id=53296

Same here.

> 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?