RESOLVED FIXED 44915
Need to handle CSSValueAuto in CSSPrimitiveValue::operator ETextAlign()
https://bugs.webkit.org/show_bug.cgi?id=44915
Summary Need to handle CSSValueAuto in CSSPrimitiveValue::operator ETextAlign()
Justin Garcia
Reported 2010-08-30 17:50:11 PDT
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).
Attachments
patch (1.78 KB, patch)
2010-08-30 18:33 PDT, Justin Garcia
darin: review-
patch (2.97 KB, patch)
2010-08-31 19:47 PDT, Justin Garcia
mitz: review+
mitz
Comment 1 2010-08-30 18:21:39 PDT
'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
Comment 2 2010-08-30 18:33:38 PDT
Darin Adler
Comment 3 2010-08-30 22:32:30 PDT
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
mitz
Comment 4 2010-08-30 22:41:53 PDT
(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
Comment 5 2010-08-30 22:44:38 PDT
(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
Comment 6 2010-08-31 19:47:40 PDT
Created attachment 66160 [details] patch Map TAAuto to -webkit-auto instead of auto.
mitz
Comment 7 2010-08-31 19:53:09 PDT
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
Comment 8 2010-08-31 23:14:26 PDT
Note You need to log in before you can comment on or make changes to this bug.