[webkit-reviews] review granted: [Bug 86746] Layout broken after cloning and re-inserting a table with a misplaced <form> : [Attachment 166895] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 3 09:38:14 PDT 2012
Darin Adler <darin at apple.com> has granted Pravin D <pravind.2k4 at gmail.com>'s
request for review:
Bug 86746: Layout broken after cloning and re-inserting a table with a
misplaced <form>
https://bugs.webkit.org/show_bug.cgi?id=86746
Attachment 166895: Patch
https://bugs.webkit.org/attachment.cgi?id=166895&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=166895&action=review
I’d like this patch better if removing the unused m_wasMalformed was done in a
separate patch. There’s no reason to mix removal of the dead code with the bug
fix.
> Source/WebCore/html/HTMLFormElement.cpp:352
> - else if (firstSuccessfulSubmitButton == 0 &&
control->isSuccessfulSubmitButton())
> + else if (!firstSuccessfulSubmitButton &&
control->isSuccessfulSubmitButton())
This reformatting and code style fix of otherwise untouched code is not a good
idea for a patch that is not otherwise changing this function.
> Source/WebCore/html/HTMLFormElement.cpp:489
> - && (static_cast<Element*>(node)->isFormControlElement()
> - || node->hasTagName(objectTag))
> - && toHTMLElement(node)->form() == this)
> + && (static_cast<Element*>(node)->isFormControlElement()
> + || node->hasTagName(objectTag))
> + && toHTMLElement(node)->form() == this)
This reformatting and code style fix of otherwise untouched code is not a good
idea for a patch that is not otherwise changing this function.
> Source/WebCore/html/HTMLFormElement.cpp:693
> + const HTMLFormElement& sourceElement = static_cast<const
HTMLFormElement&>(source);
> +
> + m_wasDemoted = sourceElement.m_wasDemoted;
I don’t think the local variables adds sufficient value here. This would read
better as a single line.
More information about the webkit-reviews
mailing list