Bug 43813

Summary: Ensure that parser doesn't attach children that have been removed by JavaScript event handlers.
Product: WebKit Reporter: Dimitri Glazkov (Google) <dglazkov>
Component: New BugsAssignee: Dimitri Glazkov (Google) <dglazkov>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aestes, hyatt, jamesr, mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on: 40742    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Dimitri Glazkov (Google) 2010-08-10 14:48:55 PDT
Ensure that parser doesn't attach children that have been removed by JavaScript event handlers.
Comment 1 Dimitri Glazkov (Google) 2010-08-10 14:53:08 PDT
Created attachment 64048 [details]
Patch
Comment 2 Adam Barth 2010-08-10 15:38:43 PDT
Comment on attachment 64048 [details]
Patch

Oh man...  This patch looks correct, but it enscarifies me.
Comment 3 James Robinson 2010-08-10 17:31:42 PDT
Looks good to me as well.  Mitz, Hyatt - have any thoughts on this?
Comment 4 Dimitri Glazkov (Google) 2010-08-11 21:43:54 PDT
I land this yes? :)
Comment 5 Adam Barth 2010-08-11 21:59:12 PDT
I think so.  :)
Comment 6 mitz 2010-08-11 22:01:51 PDT
The change looks correct to me. Andy, if you have any test cases that you’ve developed while investigating bug 40742, other than the regression test, you can verify that this patch doesn’t regress them.
Comment 7 Andy Estes 2010-08-12 15:15:11 PDT
(In reply to comment #6)
> The change looks correct to me. Andy, if you have any test cases that you’ve developed while investigating bug 40742, other than the regression test, you can verify that this patch doesn’t regress them.

Looks good to me.  I checked in several additional tests covering this behavior in http://trac.webkit.org/changeset/61744, and it looks like those tests are passing.

Dimitri, for my own edification, I'm curious how the lazy-attach optimization affects <link> elements.  I had convinced myself it wasn't when writing this patch, but I might have missed something.
Comment 8 Adam Barth 2010-08-12 15:30:25 PDT
I think we'd like to lazy attach everything.  Once you have one element that doesn't support lazy attach, you have to break out of your lazy slumber for all it's ancestors.
Comment 9 Dimitri Glazkov (Google) 2010-08-12 16:23:30 PDT
Comment on attachment 64048 [details]
Patch

Clearing flags on attachment: 64048

Committed r65281: <http://trac.webkit.org/changeset/65281>
Comment 10 Dimitri Glazkov (Google) 2010-08-12 16:23:35 PDT
All reviewed patches have been landed.  Closing bug.