RESOLVED FIXED 50396
REGRESSION(r72783): DOMActivate fires multiple times from input type=file 
https://bugs.webkit.org/show_bug.cgi?id=50396
Summary REGRESSION(r72783): DOMActivate fires multiple times from input type=file
Dimitri Glazkov (Google)
Reported 2010-12-02 09:40:55 PST
This results in a confused ChromeClient being called twice, and file upload dialog popping up multiple times in multiprocess port (Chromium and I bet WebKit2 as well).
Attachments
Patch (4.39 KB, patch)
2010-12-02 15:01 PST, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Patch (8.08 KB, patch)
2010-12-03 09:23 PST, Dimitri Glazkov (Google) 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1 2010-12-02 15:01:55 PST
Created attachment 75417 [details] Patch
Darin Adler
Comment 2 2010-12-02 15:12:47 PST
Comment on attachment 75417 [details] Patch Is there any way to make a regression test for this? Does this only affect <input type=file>?
Dimitri Glazkov (Google)
Comment 3 2010-12-02 15:19:44 PST
(In reply to comment #2) > (From update of attachment 75417 [details]) > Is there any way to make a regression test for this? Does this only affect <input type=file>? I am writing a test, sorry :) Didn't mean to mark for review.
Dimitri Glazkov (Google)
Comment 4 2010-12-03 09:23:33 PST
Created attachment 75502 [details] Patch
Dimitri Glazkov (Google)
Comment 5 2010-12-03 09:23:57 PST
Now with a layout test :)
Darin Adler
Comment 6 2010-12-03 09:36:04 PST
Comment on attachment 75502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75502&action=review > WebCore/dom/Node.cpp:1205 > + if (!node) > + return false; > + if (this == node) > + return true; > + for (ContainerNode* n = node->parentOrHostNode(); n; n = n->parentOrHostNode()) { > + if (n == this) > + return true; > + } > + return false; I like the way you wrote this, but I would have written it this way: for (Node* n = node; n; n = n->parentOrHostNode()) { if (n == this) return true; } return false; No need for the special handling of the node itself.
Dimitri Glazkov (Google)
Comment 7 2010-12-03 09:41:45 PST
(In reply to comment #6) > (From update of attachment 75502 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75502&action=review > > > WebCore/dom/Node.cpp:1205 > > + if (!node) > > + return false; > > + if (this == node) > > + return true; > > + for (ContainerNode* n = node->parentOrHostNode(); n; n = n->parentOrHostNode()) { > > + if (n == this) > > + return true; > > + } > > + return false; > > I like the way you wrote this, but I would have written it this way: > > for (Node* n = node; n; n = n->parentOrHostNode()) { > if (n == this) > return true; > } > return false; > > No need for the special handling of the node itself. I like it. I'll change before landing.
Dimitri Glazkov (Google)
Comment 8 2010-12-03 09:56:18 PST
Committed r73270: <http://trac.webkit.org/changeset/73270>
Note You need to log in before you can comment on or make changes to this bug.