Bug 43813 - Ensure that parser doesn't attach children that have been removed by JavaScript event handlers.
Summary: Ensure that parser doesn't attach children that have been removed by JavaScri...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on: 40742
Blocks:
  Show dependency tree / graph
 
Reported: 2010-08-10 14:48 PDT by Dimitri Glazkov (Google)
Modified: 2010-08-12 16:23 PDT (History)
5 users (show)

See Also:

Attachments
Patch (4.64 KB, patch)
2010-08-10 14:53 PDT, Dimitri Glazkov (Google) 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.