[webkit-reviews] review granted: [Bug 22545] Add intrinsic event support to WMLCardElement : [Attachment 25576] Initial patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 28 12:26:39 PST 2008


Sam Weinig <sam at webkit.org> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 22545: Add intrinsic event support to WMLCardElement
https://bugs.webkit.org/show_bug.cgi?id=22545

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
> +	   Fixes: https://bugs.webkit.org/show_bug.cgi?id=22545
> +
> +	   Add onenterforward/onenterbackward/ontimer support for <card>
elements.

Please note why no testing is possible right now using DRT.

> +
> +	   /* FIXME: template support
> +	   else if (m_template) {
> +	       eventHandler = m_template->eventHandler();
> +	       if (eventHandler && eventHandler->hasIntrinsicEvent(eventType))
> +		   hasIntrinsicEvent = true;
> +	   }
> +	   */

Please don't include commented out code.

> +    // FIXME Start the timer if it exists in current card
> +    /*
> +    if (eventTimer)
> +	   eventTimer->start();
> +    */
> +
> +    // FIXME: Initialize input/select  elements in this card
> +    /*
> +    Node* node = this;
> +    while (node = node->traverseNextNode()) {
> +	   if (node->hasTagName(inputTag))
> +	       static_cast<WMLInputElement*>(node)->init();
> +	   else if (node->hasTagName(selectTag))
> +	       static_cast<WMLSelectElement*>(node)->selectInitialOptions();
> +    }
> +    */
Here too.

> +}
> +
> +void WMLCardElement::parseMappedAttribute(MappedAttribute* attr)
> +{
> +    WMLIntrinsicEventType eventType = WMLIntrinsicEventUnknown;
> +
> +    if (attr->name() == onenterforwardAttr)
> +	   eventType = WMLIntrinsicEventOnEnterForward;
> +    else if (attr->name() == onenterbackwardAttr)
> +	   eventType = WMLIntrinsicEventOnEnterBackward;
> +    else if (attr->name() == ontimerAttr)
> +	   eventType = WMLIntrinsicEventOnTimer;
> +    else if (attr->name() == newcontextAttr)
> +	   m_isNewContext = (attr->value() == "true") ? true : false;
> +    else if (attr->name() == orderedAttr)
> +	   m_isOrdered = (attr->value() == "true") ? true : false;
The explicit true/false are not needed.


r=me but please address the comments.


More information about the webkit-reviews mailing list