Bug 17433 - getComputedStyle() -> clip returns empty string
: getComputedStyle() -> clip returns empty string
Status: CLOSED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2008-02-18 21:01 PST by
Modified: 2008-08-19 16:49 PST (History)


Attachments
testcase showing problem (706 bytes, text/html)
2008-02-18 21:59 PST, Garrett Smith
no flags Details
First attempt (3.97 KB, patch)
2008-05-16 00:01 PST, Rob Buis
hyatt: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-02-18 21:01:16 PST
getComputedStyle(el,'').getPropertyValue('clip')

returns empty string.

Should return a clip value, e.g "rect(10px, 10px, 0px, 0px)"
------- Comment #1 From 2008-02-18 21:59:32 PST -------
Created an attachment (id=19205) [details]
testcase showing problem
------- Comment #2 From 2008-05-16 00:01:24 PST -------
Created an attachment (id=21195) [details]
First attempt

So now we give a better CSSPrimitiveValue for the computed value of 'clip'.
What is not completely clear to me is how to treat 'auto'. Right now it returns
rect(0px, 0px, 0px, 0px), I would have expected something with at least a width
and height, also no clipping can be seen. However if I specify rect(0px, 0px, 0px, 0px) explicitly, there is clipping. I don't see how a distinction is made
between these two cases, and whether it is relevant for the computed value. Opera also returns what we return for auto, FF3 just returns 'auto'.
Cheers,

Rob.
------- Comment #3 From 2008-05-16 00:03:13 PST -------
(From update of attachment 21195 [details])
r=me
------- Comment #4 From 2008-05-16 12:15:55 PST -------
Landed in r33513.
------- Comment #5 From 2008-05-21 11:52:52 PST -------
btw this also rdar://5875146
------- Comment #6 From 2008-07-16 10:42:36 PST -------
(In reply to comment #2)
> Created an attachment (id=21195) [edit] [details]
> First attempt
> 
> So now we give a better CSSPrimitiveValue for the computed value of 'clip'.
> What is not completely clear to me is how to treat 'auto'. 

"rect(0, " + el.offsetWidth +", " + el.offsetHeight +", " + "0)"

> Right now it returns
> rect(0px, 0px, 0px, 0px), 

That's worse than returning an empty string. Before the "fix", if computedStyle.clip was the empty string, it was logical to assume that the value couldn't be read. Now it's impossible to tell what the actual computed value is where as b. Is the element really clipped, or is this the result of a bug 17433's "fix"?

> I would have expected something with at least a width
> and height, also no clipping can be seen. 

Of course.

> However if I specify rect(0px, 0px,
> 0px, 0px) explicitly, there is clipping. 

That seems to violate transivity.

> I don't see how a distinction is made
> between these two cases, and whether it is relevant for the computed value.
> Opera also returns what we return for auto, FF3 just returns 'auto'.

returning 'auto' is wrong. That is not the CSS 2.1 computed value for clip.

The computed value for clip is defined by css 2.1:
http://www.w3.org/TR/CSS21/visufx.html#clipping

| Computed value:      For rectangle values, a rectangle 
| consisting of four computed lengths; otherwise, as specified 

Returning all zeros, or "rect(0px, 0px, 0px, 0px)", is obviously inaccurate and intransitive, as you noticed.

If the computedStyle.clip is all zeros, and is used to set el.style.clip, it will result in the element being clipped. Obviously, the error is in the computedStyle.clip incorrectly returning all zeros. This can be demonstrated by an example:-

<!DOCTYPE HTML>
<html lang="en">
<head>
    <title>Clip Test</title>
</head>
<body>

<div style="position: absolute" id="el"
>will be clipped?</div>
<script>
var el = document.getElementById('el'),

  // Since readStyle.clip returns empty string in current Webkit, 
  // use a "mock" object.
    readStyle = {clip: "rect(0px, 0px, 0px, 0px)"}; 
    //readStyle = getComputedStyle(el,''),
    style = el.style;

alert(style.clip = readStyle.clip);
alert("getPropertyCSSValue('clip'): " +getComputedStyle(el,'').getPropertyCSSValue('clip'));

</script>

</body>
</html>


To solve the problem, return a correct length. This is as simple as:-

"rect(0, " + el.offsetWidth +", " + el.offsetHeight +", " + "0)"


We can also see that 'getPropertyCSSValue' does not return a Rect for clip but returns null instead. (Method  getPropertyCSSValue should also return the correct computed lengths (not all zeros).)


There is a need for a determination between "absolute" value and "computed" value in the DOM Style specs.

> 
> Rob.
> 
------- Comment #7 From 2008-07-16 22:33:36 PST -------
The value should be comma separated. I would recommend that the assertion for the unit test you make not have a hardcoded value for "rect(5px 24px 1px 12px)".

http://www.w3.org/TR/CSS21/visufx.html#clipping
| Authors should separate offset values with commas. User agents must 
| support separation with commas, but may also support separation without 
| commas, because a previous revision of this specification was ambiguous in 
| this respect.

Don't return space-separated values. Return a rect() string with comma-separated values, as the CSS2.1 specification recommends.
------- Comment #8 From 2008-07-26 22:31:33 PST -------
Garrett, could you please file a new bug report about the problems you have observed in the current implementation?
------- Comment #9 From 2008-07-26 23:15:48 PST -------
The current implementation that I have, of Safari 3, on Mac, clip returns an empty string. That is this bug, and it is described in comment 0.
------- Comment #10 From 2008-07-26 23:19:29 PST -------
A patch was landed in r33513 to address getComputedStyle returning an empty string.  You then reopened this bug report due to issues in the new results.  You should file a new bug report about those issues and this bug report should be closed.
------- Comment #11 From 2008-07-26 23:20:18 PST -------
Then I will have to download a nightly build. 
------- Comment #12 From 2008-08-19 16:23:11 PST -------
Created Bug 20454

 This bug is not FIXED. Perhaps its best marked WONTFIX?
------- Comment #13 From 2008-08-19 16:27:21 PST -------
It will be marked as FIXED as it no longer returns an empty string, which was the originally reported issue.  If we had decided to keep it returning an empty string then WONTFIX would be an appropriate resolution.  The issues in the return value will be tracked via the new bug report.  Thanks for filing it!
------- Comment #14 From 2008-08-19 16:45:03 PST -------
not really fixed. Just created a new bug.
------- Comment #15 From 2008-08-19 16:45:58 PST -------
WONTFIX, (only because I didn't find a more appropriate option)
------- Comment #16 From 2008-08-19 16:48:13 PST -------
As I mentioned, WONTFIX is not appropriate here.
------- Comment #17 From 2008-08-19 16:49:43 PST -------
Fixes to bugs occasionally introduce new bugs.  That does not mean that we have no intent to fix the original issue, nor that the original issue is unfixed.  When new bugs are introduced, they are tracked separately.  You've already filed the new bug report, so I see no need for you to keep fiddling with the status of this bug report.