[webkit-reviews] review denied: [Bug 5122] Equivalent of Mozilla's DOMContentLoaded needed : [Attachment 6950] A patch that adds support for the DOMContentLoaded event

[bugzilla-request-daemon at opendarwin.org]

Maciej Stachowiak <mjs at apple.com> has denied Maciej Stachowiak
<mjs at apple.com>'s request for review:
Bug 5122: Equivalent of Mozilla's DOMContentLoaded needed
http://bugzilla.opendarwin.org/show_bug.cgi?id=5122

Attachment 6950: A patch that adds support for the DOMContentLoaded event 
http://bugzilla.opendarwin.org/attachment.cgi?id=6950&action=edit

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
In general this change looks really good, a few small comments (also mentioned
on IRC):

- The test case doesn't need to do the fibonnacci thing, JS is single-threaded
so that doesn't really add anything.

- The test case could check a few more things. For instance, it could load an
external image or stylesheet or something, and verify that DOMContentLoaded
fires before the subresource onload event (this isn't guaranteed, but in the
layout test case it shouldn't be in cache). You could also check that the whole
document is parsed by checking for instance that the last element of the
document is there as expected.

- There's no need to specially filter this event like DOM mutation events, it
is not gonna fire often enough to need the optimization.

You can probably get rid of this comment:

+    // I think this is the right place for the DOMContentLoaded trigger, but
maybe not...

r- for these technicalities, can't wait to see the updated version.