Bug 17433 - getComputedStyle() -> clip returns empty string
: getComputedStyle() -> clip returns empty string
Status: CLOSED FIXED
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-18 21:01 PST by Garrett Smith
Modified: 2008-08-19 16:49 PDT (History)
2 users (show)

See Also:


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 PDT, Rob Buis
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Garrett Smith 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 Garrett Smith 2008-02-18 21:59:32 PST
Created attachment 19205 [details]
testcase showing problem
Comment 2 Rob Buis 2008-05-16 00:01:24 PDT
Created attachment 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 Dave Hyatt 2008-05-16 00:03:13 PDT
Comment on attachment 21195 [details]
First attempt

r=me
Comment 4 Rob Buis 2008-05-16 12:15:55 PDT
Landed in r33513.
Comment 5 David Harrison 2008-05-21 11:52:52 PDT
btw this also rdar://5875146
Comment 6 Garrett Smith 2008-07-16 10:42:36 PDT
(In reply to comment #2)
> Created an attachment (id=21195) [edit]
> 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 Garrett Smith 2008-07-16 22:33:36 PDT
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 Mark Rowe (bdash) 2008-07-26 22:31:33 PDT
Garrett, could you please file a new bug report about the problems you have observed in the current implementation?
Comment 9 Garrett Smith 2008-07-26 23:15:48 PDT
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 Mark Rowe (bdash) 2008-07-26 23:19:29 PDT
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 Garrett Smith 2008-07-26 23:20:18 PDT
Then I will have to download a nightly build. 
Comment 12 Garrett Smith 2008-08-19 16:23:11 PDT
Created Bug 20454

 This bug is not FIXED. Perhaps its best marked WONTFIX?
Comment 13 Mark Rowe (bdash) 2008-08-19 16:27:21 PDT
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 Garrett Smith 2008-08-19 16:45:03 PDT
not really fixed. Just created a new bug.
Comment 15 Garrett Smith 2008-08-19 16:45:58 PDT
WONTFIX, (only because I didn't find a more appropriate option)
Comment 16 Mark Rowe (bdash) 2008-08-19 16:48:13 PDT
As I mentioned, WONTFIX is not appropriate here.
Comment 17 Mark Rowe (bdash) 2008-08-19 16:49:43 PDT
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.