To fire each event handler registered on an SVG node once per event, Chromium needs an implementation of wasCreatedFromMarkup added to V8LazyEventListener.h that matches the one in WebKit/WebCore/bindings/js/JSLazyEventListener.h.
Created attachment 49355 [details] Patch to fix bug 35325
Comment on attachment 49355 [details] Patch to fix bug 35325 > + 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.
(In reply to comment #2) > (From update of attachment 49355 [details]) > > + 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. This is easily removed. I'll upload a new patch without it after dealing with the below. > > > + // 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 :) I'm not sure how to address this comment. I'd be happy to remove the comment. Deleting is easy. :-) Argument in favour of removing the comment: it would match the brevity of the un-commented JSLazyEventListener::wasCreatedFromMarkup() I know how the presence of JSLazyEventListener::wasCreatedFromMarkup changes the behaviour of the SVG event handling code: this method returns true if the clone of a SVG Node (into the shadow tree) includes event handlers already or if they need to be added in a second pass over the DOM. I think that this patch (without the comment) is the smallest amount of code that corrects Chromium's SVG event dispatch behaviour to match with WebKit. Please advise me further on what I need to do to make the patch acceptable. > > > r- on nitpicks.
Created attachment 49604 [details] patch for bug 35325
Comment on attachment 49604 [details] patch for bug 35325 Great, thanks! > + Reviewed by Nate Chapin For future reference, typically we just leave the "Reviewed by NOBODY (OOPS!)." in place until a patch has received an r+. The commit queue scripts will take care of replace it with the name of the person who actually approved the patch.
Comment on attachment 49604 [details] patch for bug 35325 Clearing flags on attachment: 49604 Committed r55326: <http://trac.webkit.org/changeset/55326>
All reviewed patches have been landed. Closing bug.