[webkit-reviews] review granted: [Bug 22550] Add WML <timer> element support : [Attachment 25581] Initial patch

[bugzilla-daemon at webkit.org]

Cameron Zwarich (cpst) <cwzwarich at uwaterloo.ca> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 22550: Add WML <timer> element support
https://bugs.webkit.org/show_bug.cgi?id=22550

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

------- Additional Comments from Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>
The comment

// The value of timeout must be a positive integral number, or ignore it

would probably be more clear as

// If the value of timeout is not a positive integer, ignore it

In WMLTimerElement::insertedIntoDocument(), you have both

     ASSERT(parent);

and

    if (!parent || !parent->isWMLElement())

You should decide if parent should ever be null and make a decision
accordingly. 

+    if (interval <= 0)
+	 interval = m_value.toInt();
+
+    if (interval > 0)
+	 m_timer.startOneShot(interval / 10.0f);

Can't the second conditional be an else?