Bug 137567

Summary: Computed style for clip is wrong with respect to auto
Product: WebKit Reporter: Ricky Mondello <rmondello>
Component: CSSAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, rmondello, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Test
none
Test that writes to page
none
Patch
none
Patch
none
Patch simon.fraser: review+

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>