Need to handle CSSValueAuto in CSSPrimitiveValue::operator ETextAlign()

• Component: CSS
• Status: RESOLVED FIXED

[Justin Garcia]

When we map a CSSPrimitiveValue to an ETextAlign value, we need to handle CSSValueAuto, since we map TAAUTO to CSSValueAuto when going in the opposite direction in CSSPrimitiveValue::CSSPrimitiveValue(ETextAlign e).

'auto' is not a vaild text-align value. We use TAAUTO internally for the “nameless value that acts as 'left' if 'direction' is 'ltr', 'right' if 'direction' is 'rtl'” from CSS2.1.

I’m just not sure where you’re going with this :)

[Justin Garcia]

Created attachment 65994 [details]
patch

[Darin Adler]

Comment on attachment 65994 [details]
patch

> +        No new tests because we don't appear to cast a CSSPrimitiveValue to an ETextAlign anywhere currently.

We convert a CSSPrimitiveValue to an ETextAlign in CSSStyleSelector::applyProperty's case for CSSPropertyTextAlign. You should be able to create a test case based on using text-align: auto.

> Index: WebCore.xcodeproj/project.pbxproj
> ===================================================================
> --- WebCore.xcodeproj/project.pbxproj	(revision 66441)
> +++ WebCore.xcodeproj/project.pbxproj	(working copy)
> @@ -20491,6 +20491,7 @@
>  			isa = PBXProject;
>  			buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
>  			compatibilityVersion = "Xcode 2.4";
> +			developmentRegion = English;
>  			hasScannedForEncodings = 1;
>  			knownRegions = (
>  				English,

Please do not check this project change in.

review- for lack of test case

(In reply to comment #3)
> (From update of attachment 65994 [details])
> > +        No new tests because we don't appear to cast a CSSPrimitiveValue to an ETextAlign anywhere currently.
> 
> We convert a CSSPrimitiveValue to an ETextAlign in CSSStyleSelector::applyProperty's case for CSSPropertyTextAlign. You should be able to create a test case based on using text-align: auto.

text-algin: auto is invalid and the parser rejects it. 

CSSPrimitiveValue::operator ETextAlign() currently maps -webkit-auto to TAAUTO. It would be wrong to map auto to the same value. I think the bug is in CSSPrimitiveValue::CSSPrimitiveValue(ETextAlign). I don’t understand why it maps TAAUTO back to the invalid value auto.

[Darin Adler]

(In reply to comment #4)
> I think the bug is in CSSPrimitiveValue::CSSPrimitiveValue(ETextAlign). I don’t understand why it maps TAAUTO back to the invalid value auto.

I think you are right! Should be easy to test with getComputedStyle.

[Justin Garcia]

Created attachment 66160 [details]
patch

Map TAAuto to -webkit-auto instead of auto.

Comment on attachment 66160 [details]
patch

OK. I just checked what Firefox does, and apparently it returns the modern value 'start', but for now WebKit can do this.

[Justin Garcia]

http://trac.webkit.org/changeset/66581