Bug 86574 - Avoid reparsing the style attribute when cloning elements.
Summary: Avoid reparsing the style attribute when cloning elements.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-15 21:57 PDT by Andreas Kling
Modified: 2012-05-22 11:42 PDT (History)
7 users (show)

See Also:


Attachments
Patch (19.66 KB, patch)
2012-05-15 22:19 PDT, Andreas Kling
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch (19.66 KB, patch)
2012-05-15 22:54 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch (19.71 KB, patch)
2012-05-15 23:41 PDT, Andreas Kling
gns: commit-queue-
Details | Formatted Diff | Diff
Patch (19.80 KB, patch)
2012-05-16 00:11 PDT, Andreas Kling
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2012-05-15 21:57:28 PDT
We should avoid unnecessarily reparsing the 'style' attribute when cloning DOM elements. This would help the DOM/CloneNodes performance test, and makes sense in general.
Comment 1 Andreas Kling 2012-05-15 22:19:37 PDT
Created attachment 142147 [details]
Patch

Let's see how EWS feels about this..
Comment 2 Early Warning System Bot 2012-05-15 22:39:42 PDT
Comment on attachment 142147 [details]
Patch

Attachment 142147 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12709313
Comment 3 Build Bot 2012-05-15 22:42:53 PDT
Comment on attachment 142147 [details]
Patch

Attachment 142147 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12706555
Comment 4 Early Warning System Bot 2012-05-15 22:50:20 PDT
Comment on attachment 142147 [details]
Patch

Attachment 142147 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12720031
Comment 5 Andreas Kling 2012-05-15 22:54:13 PDT
Created attachment 142156 [details]
Patch
Comment 6 Andreas Kling 2012-05-15 23:41:09 PDT
Created attachment 142165 [details]
Patch
Comment 7 Gustavo Noronha (kov) 2012-05-16 00:03:00 PDT
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12720058
Comment 8 Build Bot 2012-05-16 00:03:01 PDT
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12721015
Comment 9 Build Bot 2012-05-16 00:05:14 PDT
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12720055
Comment 10 Early Warning System Bot 2012-05-16 00:06:42 PDT
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12718078
Comment 11 Andreas Kling 2012-05-16 00:11:13 PDT
Created attachment 142168 [details]
Patch

From the making-this-harder-than-it-needs-to-be department..
Comment 12 Antti Koivisto 2012-05-16 05:25:23 PDT
Comment on attachment 142168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142168&action=review

r=me

> Source/WebCore/dom/Element.cpp:2109
> +    // Transfer additional data if the we are cloning from another element of the same type.
> +    if (tagQName() == other.tagQName())
> +        copyNonAttributePropertiesFromElement(other);

Would it be better to have a different function for cloning to an element with a different tag name? That seems like a quite different case (why do we do it?) from regular cloning.

> Source/WebCore/dom/StyledElement.h:36
> +enum ShouldReparseStyleAttribute { DoNotReparseStyleAttribute = 0, ReparseStyleAttribute = 1 };

I would scope this to StyledElement.
Comment 13 Andreas Kling 2012-05-16 12:31:27 PDT
Committed r117323: <http://trac.webkit.org/changeset/117323>