Bug 23668 - CSS 'content' attribute does not work with window.getComputedStyle
Summary: CSS 'content' attribute does not work with window.getComputedStyle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 525.x (Safari 3.2)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2009-01-31 17:35 PST by exfed
Modified: 2011-02-24 08:59 PST (History)
6 users (show)

See Also:


Attachments
Test case (207 bytes, text/html)
2009-02-06 06:32 PST, David Kilzer (:ddkilzer)
no flags Details
Patch (12.88 KB, patch)
2011-02-23 12:42 PST, Emil A Eklund
simon.fraser: review-
Details | Formatted Diff | Diff
Patch (11.66 KB, patch)
2011-02-23 14:03 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description exfed 2009-01-31 17:35:53 PST
<div style="content: 'string'"
onclick="alert([window.getComputedStyle(this, null).getPropertyValue('content'),this.style.content])">

  displays ",string", correct should be "string,string";

</div>
Comment 1 exfed 2009-02-04 08:56:47 PST
testcase:
http://zel.pl/a.htm
Comment 2 Mark Rowe (bdash) 2009-02-06 03:20:40 PST
<rdar://problem/6562788>
Comment 3 David Kilzer (:ddkilzer) 2009-02-06 06:32:39 PST
Created attachment 27395 [details]
Test case
Comment 4 Eric Seidel (no email) 2009-02-06 10:45:15 PST
Seems like we're missing lots of these.  Content in particular:
http://trac.webkit.org/browser/trunk/WebCore/css/CSSComputedStyleDeclaration.cpp#L1206

This would be a trivial bug to fix for someone.  Most important would be to write a nice test case, ideally one which tests a whole bunch of the missing properties, and is in the newer fast/js/ testing style.
Comment 5 Emil A Eklund 2011-02-23 12:42:52 PST
Created attachment 83526 [details]
Patch

Implements getComputedStyle for the content, counter, outline-offset, background-position-x and background-position-y properties.

To correctly return content containing counters I added a new type to CSSPrimitiveValue, CSS_COUNTER_NAME. I'm not convinced this is the right thing to do but was the best option I could come up with. Any suggestions would be welcome.
Comment 6 Eric Seidel (no email) 2011-02-23 13:08:51 PST
Comment on attachment 83526 [details]
Patch

LGTM.
Comment 7 Eric Seidel (no email) 2011-02-23 13:09:28 PST
Comment on attachment 83526 [details]
Patch

Since you seem doubtful on the CSS_COUNTER_NAME thing I'll leave that for someone else to comment on.
Comment 8 Simon Fraser (smfr) 2011-02-23 13:40:33 PST
Comment on attachment 83526 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:730
> +static PassRefPtr<CSSValue> contentToCSSValue(PassRefPtr<RenderStyle> style)

The argument can be a const RenderStyle*

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:748
> +static PassRefPtr<CSSValue> counterToCSSValue(PassRefPtr<RenderStyle> style, int propertyID)

Ditto.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:852
> +        case CSSPropertyBackgroundRepeatX:
> +            return CSSPrimitiveValue::create(style->backgroundRepeatX());
> +        case CSSPropertyBackgroundRepeatY:
> +            return CSSPrimitiveValue::create(style->backgroundRepeatY());

There are no individual CSS properties for background-repeat-x and y (http://www.w3.org/TR/css3-background/#the-background-repeat), so I don't think we should be supporting computed style for them.
Comment 9 Emil A Eklund 2011-02-23 14:03:15 PST
Created attachment 83545 [details]
Patch

Thanks Simon!
I made the changes you suggested and removed the background-repeat-x|y properties.
Please take another look when you get a chance.
Comment 10 WebKit Commit Bot 2011-02-24 08:56:55 PST
The commit-queue encountered the following flaky tests while processing attachment 83545 [details]:

http/tests/xmlhttprequest/failed-auth.html bug 51835 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 11 WebKit Commit Bot 2011-02-24 08:59:31 PST
Comment on attachment 83545 [details]
Patch

Clearing flags on attachment: 83545

Committed r79574: <http://trac.webkit.org/changeset/79574>
Comment 12 WebKit Commit Bot 2011-02-24 08:59:35 PST
All reviewed patches have been landed.  Closing bug.