Bug 22545 - Add intrinsic event support to WMLCardElement
Summary: Add intrinsic event support to WMLCardElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 20393
  Show dependency treegraph
 
Reported: 2008-11-28 12:12 PST by Nikolas Zimmermann
Modified: 2008-11-28 13:53 PST (History)
0 users

See Also:


Attachments
Initial patch (9.39 KB, patch)
2008-11-28 12:17 PST, Nikolas Zimmermann
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2008-11-28 12:12:07 PST
Handle onenterforward/onenterbackward/ontimer support for WMLCardElement.
Comment 1 Nikolas Zimmermann 2008-11-28 12:17:16 PST
Created attachment 25576 [details]
Initial patch
Comment 2 Sam Weinig 2008-11-28 12:26:39 PST
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.
Comment 3 Nikolas Zimmermann 2008-11-28 13:26:43 PST
(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.

Comment 4 Nikolas Zimmermann 2008-11-28 13:53:52 PST
Landed in r38833.