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

Dimitri Glazkov (Google)
Reported 2010-08-10 14:48:55 PDT
Ensure that parser doesn't attach children that have been removed by JavaScript event handlers.
Attachments
Patch (4.64 KB, patch)
2010-08-10 14:53 PDT, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1 2010-08-10 14:53:08 PDT
Created attachment 64048 [details] Patch
Adam Barth
Comment 2 2010-08-10 15:38:43 PDT
Comment on attachment 64048 [details] Patch Oh man... This patch looks correct, but it enscarifies me.
James Robinson
Comment 3 2010-08-10 17:31:42 PDT
Looks good to me as well. Mitz, Hyatt - have any thoughts on this?
Dimitri Glazkov (Google)
Comment 4 2010-08-11 21:43:54 PDT
I land this yes? :)
Adam Barth
Comment 5 2010-08-11 21:59:12 PDT
I think so. :)
mitz
Comment 6 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.
Andy Estes
Comment 7 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.
Adam Barth
Comment 8 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.
Dimitri Glazkov (Google)
Comment 9 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>
Dimitri Glazkov (Google)
Comment 10 2010-08-12 16:23:35 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.