Bug 102120

Summary: Move inline style logic from ElementAttributeData to StyledElement.
Product: WebKit Reporter: Andreas Kling <kling>
Component: DOMAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, koivisto, macpherson, menard, mifenton, simon.fraser, webkit-ews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Scandinavian patch
webkit-ews: commit-queue-
Meh patch koivisto: review+, koivisto: commit-queue-

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