Bug 137567 - Computed style for clip is wrong with respect to auto
Summary: Computed style for clip is wrong with respect to auto
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-09 13:26 PDT by Ricky Mondello
Modified: 2014-10-09 17:19 PDT (History)
4 users (show)

See Also:

Attachments
Test (395 bytes, text/html)
2014-10-09 14:09 PDT, Ricky Mondello 		no flags Details
Test that writes to page (471 bytes, text/html)
2014-10-09 15:58 PDT, Dean Jackson 		no flags Details
Patch (5.18 KB, patch)
2014-10-09 16:30 PDT, Dean Jackson 		no flags Details | Formatted Diff | Diff
Patch (5.93 KB, patch)
2014-10-09 16:49 PDT, Dean Jackson 		no flags Details | Formatted Diff | Diff
Patch (5.89 KB, patch)
2014-10-09 17:12 PDT, Dean Jackson 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (2) 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 Ricky Mondello 2014-10-09 13:26:50 PDT 
For the following markup:

    <div id="test" style="width: 300px; height: 300px; background-color: green; clip: rect(auto, auto, auto, auto);">

The following script:

    var testRect = document.getElementById("test");
    var clipStyle = getComputedStyle(testRect).clip;
    console.log(clipStyle);

Logs:

    rect(0px, 0px, 0px, 0px)

It should log:

    rect(auto, auto, auto, auto)

I've attached a test page.
Comment 1 Radar WebKit Bug Importer 2014-10-09 13:28:14 PDT 
<rdar://problem/18601021>
Comment 2 Ricky Mondello 2014-10-09 14:09:38 PDT 
Created attachment 239568 [details]
Test
Comment 3 Dean Jackson 2014-10-09 15:58:42 PDT 
Created attachment 239577 [details]
Test that writes to page
Comment 4 Dean Jackson 2014-10-09 16:30:19 PDT 
Created attachment 239578 [details]
Patch
Comment 5 Brent Fulgham 2014-10-09 16:34:03 PDT 
Comment on attachment 239578 [details]
Patch

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

I think we have some copy/paste errors here.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2760
> +            const Length& bottom = style->clip().right();

Is right == bottom here? :-\

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2766
> +            const Length& left = style->clip().right();

Er. left == right? Copy/paste error?
Comment 6 Dean Jackson 2014-10-09 16:35:20 PDT 
SO EMBARRASS.
Comment 7 Dean Jackson 2014-10-09 16:49:52 PDT 
Created attachment 239579 [details]
Patch
Comment 8 Simon Fraser (smfr) 2014-10-09 16:54:10 PDT 
Comment on attachment 239579 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2752
> +            const Length& top = style->clip().top();
> +            if (top.isAuto())
> +                rect->setTop(cssValuePool().createIdentifierValue(CSSValueAuto));
> +            else
> +                rect->setTop(zoomAdjustedPixelValue(top.value(), style.get()));

Would prefer a little inline function which checks for auto.
Comment 9 Dean Jackson 2014-10-09 17:12:00 PDT 
Created attachment 239583 [details]
Patch
Comment 10 Dean Jackson 2014-10-09 17:19:22 PDT 
Committed r174543: <http://trac.webkit.org/changeset/174543>