Summary: | REGRESSION(r72783): DOMActivate fires multiple times from input type=file | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 49986 | ||||||||
Attachments: |
|
Description
Dimitri Glazkov (Google)
2010-12-02 09:40:55 PST
Created attachment 75417 [details]
Patch
Comment on attachment 75417 [details]
Patch
Is there any way to make a regression test for this? Does this only affect <input type=file>?
(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. Created attachment 75502 [details]
Patch
Now with a layout test :) 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. (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. Committed r73270: <http://trac.webkit.org/changeset/73270> |