[webkit-reviews] review denied: [Bug 34156] Incorrect boolean expression in isMailBlockquote() (WebCore/htmlediting.cpp) : [Attachment 47391] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Roland Steiner
<rolandsteiner at chromium.org>'s request for review:
Bug 34156: Incorrect boolean expression in isMailBlockquote()
(WebCore/htmlediting.cpp)
https://bugs.webkit.org/show_bug.cgi?id=34156

Attachment 47391: Patch
https://bugs.webkit.org/attachment.cgi?id=47391&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   Could potentially lead to a crash if applied to an attribute named
"blockquote",

That's incorrect. The Node::hasTagName will only return true for an Element.
You can look at the implementation to double-check this.

> +	   or an incorrect result if applied to a different element that has an
attribute "cite".

I don't understand this claim.

> +	   No new tests. (minor code change)

If the "cite" case existed, you could probably construct a test case for it.

> -    if (!node || (!node->isElementNode() &&
!node->hasTagName(blockquoteTag)))
> +    if (!node || !node->isElementNode() || !node->hasTagName(blockquoteTag))

>	   return false;

The isElementNode check is entirely unnecessary and can be removed. That's
probably the best fix.