[webkit-reviews] review denied: [Bug 26140] Implement onreadystatechange event handler for Documents : [Attachment 70499] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied James Simonsen
<simonjam at chromium.org>'s request for review:
Bug 26140: Implement onreadystatechange event handler for Documents
https://bugs.webkit.org/show_bug.cgi?id=26140

Attachment 70499: Patch
https://bugs.webkit.org/attachment.cgi?id=70499&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=70499&action=review

> WebCore/dom/Document.cpp:1900
> -void Document::implicitOpen()
> +void Document::implicitOpen(bool removeExistingEventListeners)
>  {
>      cancelParsing();
>  
> +    if (removeExistingEventListeners)
> +	   removeAllEventListeners();

Why is adding a boolean better than calling removeAllEventListeners in
Document::open?

> WebCore/dom/Document.h:536
> -    void implicitOpen();
> +    void implicitOpen(bool removeExistingEventListeners);

This seems like a poor change. Normally if we are just passing in a boolean
constant we use an enum instead so it’s named and can be understood at the call
site. But in this case, it seems the caller can just call
removeAllEventListeners.