Bug 5122

Summary: Need an equivalent of Mozilla's DOMContentLoaded event
Product: WebKit Reporter: Simon Willison <swillison>
Component: DOMAssignee: Mark Rowe (bdash) <mrowe>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ap, gavin.sharp, ian, mrowe, pumpkingod, rvamerongen, webkit
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://dean.edwards.name/weblog/2005/09/busted/
Bug Depends on: 7693    
Bug Blocks:    
Attachments:
Description Flags
A patch that adds support for the DOMContentLoaded event
mjs: review-
Updated patch
mjs: review-
Updated patch using Event and with faster layout test none

Description Simon Willison 2005-09-24 17:13:14 PDT
It's extremely desirable to set up a JavaScript function to be executed as soon
as the DOM has loaded. window.onload is no good as it waits for all images on
the page to load as well.

Workarounds for this exist for both Mozilla and IE, as documented here:
http://dean.edwards.name/weblog/2005/09/busted/

A way of achieving this in Safari would be very welcome.
Comment 1 Alexey Proskuryakov 2006-02-07 12:25:13 PST
Actually, script defer, mentioned in the blog entry, does work in both Safari and Firefox. Try replacing init() with alert(0) - it does pop up at the expected moment. So, some other issue may be preventing the code from working.

Some comments to the blog entry indicate that DOMContentLoaded is substantially different from defer, though.
Comment 2 Joost de Valk (AlthA) 2006-02-13 15:56:24 PST
Reassigning to webkit-unassigned, to make sure more people see this.
Comment 3 Joost de Valk (AlthA) 2006-02-15 15:26:55 PST
Discussed this on IRC with ggaren and mjs, and it is a valid request.
Comment 4 Beau Hartshorne 2006-03-01 11:49:14 PST
There's a good discussion of defer here:
http://www.davidflanagan.com/blog/2006_01.html
Comment 5 Daniel Peebles 2006-03-08 20:28:47 PST
I'm working on it. Just writing a testcase for it, although I can only base my idea for its 'right' behavior on that of firefox... at the moment I consider it right if it gets triggered independently of external resources, and onload occurs after DOMContentLoaded... any other restrictions you can think of?
Comment 6 Daniel Peebles 2006-03-08 22:22:34 PST
Created attachment 6950 [details]
A patch that adds support for the DOMContentLoaded event 

Just added a few lines here and there. It appears to work the way it should, but "should" isn't very well defined. It does get triggered when the document finishes parsing, which appears to be the generally accepted contract for the event.
Comment 7 Maciej Stachowiak 2006-03-09 00:42:58 PST
Comment on attachment 6950 [details]
A patch that adds support for the DOMContentLoaded event 

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.
Comment 8 Daniel Peebles 2006-03-09 03:13:16 PST
(In reply to comment #7)
> (From update of attachment 6950 [details] [edit])
> 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.
> 

ran into a really strange bug while trying to fix those, so I'll probably have it in by tomorrow :)
Comment 9 Daniel Peebles 2006-03-09 21:54:25 PST
My implementation of DOMContentLoaded causes odd behavior in DumpRenderTree, so I've filed another bug for that (7693.) I'm seeing if I can fix that right now :-/
Comment 10 Arthur Langereis 2007-08-20 07:37:06 PDT
With bug 7693 resolved for over a year now, can you try to get DOMContentLoaded in again? I and several others would love to see this feature added in time for 3.0.

The current workaround for this is an ugly sniffer timer that checks the readyState property, a real event would be great. When Safari adds this, the relevant non-IE browsers will all support this. Sorry for the spam, it's just been very quiet here.
Comment 11 Daniel Peebles 2007-08-20 07:51:15 PDT
Sorry, have been out of the loop for a while. I'll test my patch to make sure everything works and submit an updated patch in the next few days.

(In reply to comment #10)
> With bug 7693 resolved for over a year now, can you try to get DOMContentLoaded
> in again? I and several others would love to see this feature added in time for
> 3.0.
> 
> The current workaround for this is an ugly sniffer timer that checks the
> readyState property, a real event would be great. When Safari adds this, the
> relevant non-IE browsers will all support this. Sorry for the spam, it's just
> been very quiet here.
> 
Comment 12 Mark Rowe (bdash) 2007-09-30 03:14:16 PDT
I'm working on cleaning up this patch.
Comment 13 Mark Rowe (bdash) 2007-09-30 03:22:58 PDT
Created attachment 16468 [details]
Updated patch
Comment 14 Maciej Stachowiak 2007-09-30 03:34:19 PDT
Comment on attachment 16468 [details]
Updated patch

DOMContentLoaded should probably be a generic event, not a MutationEvent.
Comment 15 Mark Rowe (bdash) 2007-09-30 03:38:17 PDT
Created attachment 16469 [details]
Updated patch using Event and with faster layout test
Comment 16 Maciej Stachowiak 2007-09-30 03:41:41 PDT
Comment on attachment 16469 [details]
Updated patch using Event and with faster layout test

r=me for feature-branch
Comment 17 Eric Seidel (no email) 2007-09-30 11:09:39 PDT
This is a GIT diff, someone with GIT (likely bdash) will need to land it on fb.
Comment 18 Mark Rowe (bdash) 2007-09-30 14:58:30 PDT
Eric, svn-apply handles git diffs just fine.
Comment 19 Mark Rowe (bdash) 2007-09-30 15:29:28 PDT
That said, I'll hold onto this and land this a bit later on after trunk opens up.  I have a slightly tweaked version locally, so please do not land the patch on this bug :)
Comment 20 Eric Seidel (no email) 2007-10-07 02:08:32 PDT
Comment on attachment 16469 [details]
Updated patch using Event and with faster layout test

Marking this patch as obsolete to remove it from the review queue.  (Since it's obsoleted by a patch on bdash's HD.)
Comment 21 Eric Seidel (no email) 2007-10-07 02:09:26 PDT
Comment on attachment 16469 [details]
Updated patch using Event and with faster layout test

Bah, commit queue.  And that didn't do it.  Clearing flag.
Comment 22 Eric Seidel (no email) 2007-10-07 02:10:08 PDT
Comment on attachment 16469 [details]
Updated patch using Event and with faster layout test

Also, (somewhat unrelated) I've heard rumors that "trunk" will be "feature-branch" soon anyway.
Comment 23 Mark Rowe (bdash) 2007-10-07 07:47:28 PDT
Landed in r26101.