Bug 22497 - Parse turns for <angle> units
Summary: Parse turns for <angle> units
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2008-11-25 14:17 PST by Dean Jackson
Modified: 2009-03-02 11:51 PST (History)
3 users (show)

See Also:


Attachments
Patch for 'turn' CSS angle unit type + testcases (20.41 KB, patch)
2008-11-27 17:56 PST, Dean Jackson
no flags Details | Formatted Diff | Diff
updated patch (22.75 KB, patch)
2008-12-01 22:47 PST, Dean Jackson
mitz: review-
Details | Formatted Diff | Diff
update with (hopefully) correct enum vals (18.83 KB, patch)
2008-12-02 13:20 PST, Dean Jackson
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2008-11-25 14:17:50 PST
CSS says <angle> should accept "turn" units

<rdar://problem/6217769>
Comment 1 Dean Jackson 2008-11-27 17:56:16 PST
Created attachment 25562 [details]
Patch for 'turn' CSS angle unit type + testcases
Comment 2 mitz 2008-12-01 20:58:57 PST
Comment on attachment 25562 [details]
Patch for 'turn' CSS angle unit type + testcases

Is it okay for the values in CSSPrimitiveValue.h to mismatch the values in CSSPrimitiveValue.idl?
Comment 3 Dean Jackson 2008-12-01 21:08:56 PST
No, it should be the same. I'll update the patch

Comment 4 Dean Jackson 2008-12-01 22:47:13 PST
Created attachment 25667 [details]
updated patch

Updated patch for mitz's comment. Also updated test for Window properties since it changed with the new value for CSS_TURN.
Comment 5 mitz 2008-12-01 22:57:55 PST
Comment on attachment 25667 [details]
updated patch

The values in the .idl come from <http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSPrimitiveValue> and I don't think they can be changed. Since the turn unit is not specified (yet?), I think it should be left out of the IDL completely and assigned a UnitType value that does not clash with the IDL-specified ones. It seems like the 100-and-higher range has been used for such "units".

Sorry about not thinking about all that before making my previous comment :(
Comment 6 Dean Jackson 2008-12-02 12:49:34 PST
DOH! You're right. The reason I shoved it in the list was I saw a lot of code in the CSS engine that checks for < and > various types. I don't know why the engine thinks the order of those enums is important, but I thought it would be less risky to put a new unit in the "right" place.

I'll put it at the end (100+) and see if anything breaks. The unit is defined in CSS3 but not CSS 2.1.

Comment 7 Dean Jackson 2008-12-02 13:20:53 PST
Created attachment 25679 [details]
update with (hopefully) correct enum vals

Changed the enum values back to the specified values, and added TURN as 100+.
Comment 8 mitz 2008-12-02 16:45:35 PST
Comment on attachment 25679 [details]
update with (hopefully) correct enum vals

r=me, but as discussed in IRC, please don't check in the changed to the IDL.
Comment 9 Dean Jackson 2008-12-03 12:47:35 PST
Committed without the IDL change. This also meant the Window DOM testcase didn't need updating.

	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/wtf/MathExtras.h
	M	LayoutTests/ChangeLog
	M	LayoutTests/transforms/2d/transform-2d-expected.txt
	M	LayoutTests/transforms/2d/transform-2d.html
	M	LayoutTests/transforms/transform-value-types-expected.txt
	M	LayoutTests/transforms/transform-value-types.html
	M	WebCore/ChangeLog
	M	WebCore/css/CSSGrammar.y
	M	WebCore/css/CSSParser.cpp
	M	WebCore/css/CSSPrimitiveValue.cpp
	M	WebCore/css/CSSPrimitiveValue.h
	M	WebCore/css/CSSStyleSelector.cpp
	M	WebCore/css/tokenizer.flex
Committed r38959