Bug 85500

Summary: REGRESSION(r111387): CSSOM representation of 'background-image' values should be CSSPrimitiveValue.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: koivisto, macpherson, menard, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patçh
koivisto: review-
Patch v2 koivisto: review+

Description Andreas Kling 2012-05-03 08:04:53 PDT
This commit <http://trac.webkit.org/changeset/111387> broke an internal client of CSSImageValue.

<rdar://problem/11305710>
Comment 1 Andreas Kling 2012-05-03 08:54:06 PDT
Created attachment 140025 [details]
Patçh
Comment 2 Antti Koivisto 2012-05-03 09:03:06 PDT
Comment on attachment 140025 [details]
Patçh

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

> Source/WebCore/css/CSSImageValue.cpp:114
> +    if (!m_cssomWrapper)
> +        m_cssomWrapper = CSSPrimitiveValue::create(m_url, CSSPrimitiveValue::CSS_URI);
> +    return m_cssomWrapper;

This will need to have m_isCSSOMSafe bit set or you will hit assert.
Comment 3 Antti Koivisto 2012-05-03 09:23:59 PDT
Comment on attachment 140025 [details]
Patçh

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

> Source/WebCore/css/CSSImageValue.h:62
> +    mutable RefPtr<CSSValue> m_cssomWrapper;

We don't cache the value wrapper to the underlying value elsewhere. This shouldn't be necessary as we have CSSStyleDeclaration level cache for keeping the identity.
Comment 4 Andreas Kling 2012-05-03 10:16:27 PDT
Created attachment 140042 [details]
Patch v2
Comment 5 Antti Koivisto 2012-05-03 10:42:04 PDT
Comment on attachment 140042 [details]
Patch v2

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

r=me

> Source/WebCore/css/CSSImageValue.cpp:114
> +    RefPtr<CSSPrimitiveValue> uriValue = CSSPrimitiveValue::create(m_url, CSSPrimitiveValue::CSS_URI);
> +    uriValue->setCSSOMSafe();
> +    return uriValue.release();

This could use a comment about why CSSImageValue gets turned into CSSPrimitiveValue.
Comment 6 Andreas Kling 2012-05-03 11:14:34 PDT
Committed r115991: <http://trac.webkit.org/changeset/115991>