[webkit-reviews] review denied: [Bug 104191] Implement matching cue by the class name with ::cue pseudo element : [Attachment 178938] Proposed fix 0.6

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 13 18:04:55 PST 2012


Antti Koivisto <koivisto at iki.fi> has denied Dima Gorbik <dgorbik at apple.com>'s
request for review:
Bug 104191: Implement matching cue by the class name with ::cue pseudo element
https://bugs.webkit.org/show_bug.cgi?id=104191

Attachment 178938: Proposed fix 0.6
https://bugs.webkit.org/attachment.cgi?id=178938&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=178938&action=review


Looks good otherwise.

> Source/WebCore/html/track/TextTrackCue.cpp:494
> +    for (Node* child = root->firstChild(); child; child =
NodeTraversal::next(child, root))
> +	   static_cast<Element*>(child)->setIsWebVTTNode(true);

This is still unsafe case. If someone adds a non Element (like Text) to the
WebVTT tree you will have memory corruption. At minimum you should use
toElement() casting function which includes assert. But really using the new
ElementTraversal interface is the way to go.


More information about the webkit-reviews mailing list