Bug 102120 - Move inline style logic from ElementAttributeData to StyledElement.
Summary: Move inline style logic from ElementAttributeData to StyledElement.
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-11-13 12:32 PST by Andreas Kling
Modified: 2012-11-13 22:03 PST (History)
8 users (show)

See Also:


Attachments
Scandinavian patch (22.20 KB, patch)
2012-11-13 12:33 PST, Andreas Kling
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Meh patch (23.40 KB, patch)
2012-11-13 12:56 PST, Andreas Kling
koivisto: review+
koivisto: commit-queue-
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-11-13 12:32:28 PST
ElementAttributeData should be dumber.
Comment 1 Andreas Kling 2012-11-13 12:33:21 PST
Created attachment 173958 [details]
Scandinavian patch
Comment 2 Early Warning System Bot 2012-11-13 12:43:45 PST
Comment on attachment 173958 [details]
Scandinavian patch

Attachment 173958 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14826284
Comment 3 Early Warning System Bot 2012-11-13 12:54:23 PST
Comment on attachment 173958 [details]
Scandinavian patch

Attachment 173958 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14832025
Comment 4 Andreas Kling 2012-11-13 12:56:43 PST
Created attachment 173959 [details]
Meh patch
Comment 5 Antti Koivisto 2012-11-13 18:06:04 PST
Comment on attachment 173959 [details]
Meh patch

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

> Source/WebCore/dom/StyledElement.cpp:146
> +    // For some reason, we allow CSSOM wrappers for inline style declarations to outlive the element.
> +    if (PropertySetCSSStyleDeclaration* cssomWrapper = inlineStyleCSSOMWrapper())
> +        cssomWrapper->clearParentElement();

The comment doesn't seem that helpful. Do you mean that the wrapper should keep the element alive?

> Source/WebCore/dom/StyledElement.cpp:151
> -    return ensureInlineStyle()->ensureInlineCSSStyleDeclaration(this);
> +    return ensureMutableInlineStyle()->ensureInlineCSSStyleDeclaration(this);

Would be good to move the wrapper ownership to StyledElement too at some point. The current propertySetCSSOMWrapperMap hackery is silly.
Comment 6 Andreas Kling 2012-11-13 21:39:56 PST
Committed r134539: <http://trac.webkit.org/changeset/134539>
Comment 7 Simon Fraser (smfr) 2012-11-13 22:03:07 PST
This broke Windows http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29