[webkit-reviews] review granted: [Bug 22549] Add WML <do> element support. : [Attachment 25579] Updated patch

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 22549: Add WML <do> element support.
https://bugs.webkit.org/show_bug.cgi?id=22549

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

------- Additional Comments from Holger Freyther <zecke at selfish.org>

> +2008-11-28  Nikolas Zimmermann  <nikolas.zimmermann at torchmobile.com>
> +
> +	   Reviewed by Holger Freyther

^^ haha ;)

> +void WMLDoElement::defaultEventHandler(Event* event)

I assume that you look at the right events?! But if you don't I'm pretty sure
you will find that out sooner than later.

> +
> +    Node* parent = parentNode();
> +    ASSERT(parent);
> +
> +    if (!parent || !parent->isWMLElement())
> +	   return;

Before landing decide if the parentNode might be zero or not and then remove
the assert or the !parent from the if?

> +    bool m_isOption;

maybe call it m_isOptional? but that is optional...


from a grep in the source tree it looks like you uncommented all doTag users...