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 		gustavo: 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
Show Obsolete (3) 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 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>