[webkit-reviews] review denied: [Bug 35325] [v8] Missing wasCreatedFromMarkup fn in V8LazyEventListener.h : [Attachment 49355] Patch to fix bug 35325

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 25 10:37:43 PST 2010


Nate Chapin <japhet at chromium.org> has denied Robert Kroeger
<rjkroege at chromium.org>'s request for review:
Bug 35325: [v8] Missing wasCreatedFromMarkup fn in V8LazyEventListener.h
https://bugs.webkit.org/show_bug.cgi?id=35325

Attachment 49355: Patch to fix bug 35325
https://bugs.webkit.org/attachment.cgi?id=49355&action=review

------- Additional Comments from Nate Chapin <japhet at chromium.org>
> +	   Patch was tested and passed the Chromium try bots.

Try bot info shouldn't go in the ChangeLog. If you're going to include it,
leave it in the bug.

> +	   // Needed to match JSC semantics inside shared SVG code.
> +	   virtual bool wasCreatedFromMarkup() const { return true; }
> +

This comment doesn't really help me understand why this change is necessary. 
It would probably be better off explaining why either set of javascript
bindings needs to override EventListener::wasCreatedFromMarkup(), rather than
just saying that we're matching JSC. Alternatively, you could present a case
why a comment isn't necessary here :)


r- on nitpicks.


More information about the webkit-reviews mailing list