Bug 17433

Summary: getComputedStyle() -> clip returns empty string
Product: WebKit Reporter: Garrett Smith <xk1t>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: CLOSED FIXED    
Severity: Normal CC: bdakin, mrowe
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
testcase showing problem
none
First attempt hyatt: review+

Garrett Smith
Reported 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)"
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+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Garrett Smith
Comment 1 2008-02-18 21:59:32 PST
Created attachment 19205 [details] testcase showing problem
Rob Buis
Comment 2 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.
Dave Hyatt
Comment 3 2008-05-16 00:03:13 PDT
Comment on attachment 21195 [details] First attempt r=me
Rob Buis
Comment 4 2008-05-16 12:15:55 PDT
Landed in r33513.
David Harrison
Comment 5 2008-05-21 11:52:52 PDT
btw this also rdar://5875146
Garrett Smith
Comment 6 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. >
Garrett Smith
Comment 7 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.
Mark Rowe (bdash)
Comment 8 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?
Garrett Smith
Comment 9 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.
Mark Rowe (bdash)
Comment 10 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.
Garrett Smith
Comment 11 2008-07-26 23:20:18 PDT
Then I will have to download a nightly build.
Garrett Smith
Comment 12 2008-08-19 16:23:11 PDT
Created Bug 20454 This bug is not FIXED. Perhaps its best marked WONTFIX?
Mark Rowe (bdash)
Comment 13 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!
Garrett Smith
Comment 14 2008-08-19 16:45:03 PDT
not really fixed. Just created a new bug.
Garrett Smith
Comment 15 2008-08-19 16:45:58 PDT
WONTFIX, (only because I didn't find a more appropriate option)
Mark Rowe (bdash)
Comment 16 2008-08-19 16:48:13 PDT
As I mentioned, WONTFIX is not appropriate here.
Mark Rowe (bdash)
Comment 17 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.
Note You need to log in before you can comment on or make changes to this bug.