Bug 22545 – Add intrinsic event support to WMLCardElement

• Component: XML
• Status: RESOLVED FIXED

[Nikolas Zimmermann]

Handle onenterforward/onenterbackward/ontimer support for WMLCardElement.

[Nikolas Zimmermann]

Created attachment 25576 [details]
Initial patch

[Sam Weinig]

Comment on attachment 25576 [details]
Initial patch

> +        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.

[Nikolas Zimmermann]

(In reply to comment #2)
> (From update of attachment 25576 [details] [review])
> > +        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.
I wouldn't need scripting to test this, but a working <go> implementation, because these events use the <go> element internally.

> > +
> > +        /* 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.
I've included comment code like this everywhile in wml/. I've already landed similar patches. It helps a lot while merging in the original WML patch. Remember, this is not 'new' work. I'm just reworking the original TorchMobile WML support patch.

> > +    // 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.
Same reason, why it should stay :-)

> 
> > +}
> > +
> > +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.
Ok, will fix.

Thanks for the review.

[Nikolas Zimmermann]

Landed in r38833.