[Webkit-unassigned] [Bug 86703] [CSS3 Backgrounds and Borders] Refactor background-position parsing code for 2 values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=86703





--- Comment #4 from Uday Kiran <udaykiran at motorola.com>  2012-05-18 04:13:37 PST ---
(From update of attachment 142412)
View in context: https://bugs.webkit.org/attachment.cgi?id=142412&action=review

>>> Source/WebCore/css/CSSParser.cpp:3312
>>> +    case 2:
>> 
>> I don't really understand why you picked up a switch - case. A very simple state machine (see the previous code) was a better design and for 4 values shouldn't be too complicated to implement.
>> 
>> Here you end up with some massive switch that will likely increase as you implement more of the syntax.
> 
> See global comment.

If you see a draft patch from bug 37514 (https://bugs.webkit.org/attachment.cgi?id=138085&action=review) which I had uploaded, it implements case values 3 and 4.
It is not much code since we need to consider only few cases as offset must be preceded by keyword.
1. keyword offset keyword offset for four values. 
2. keyword keyword offset, keyword offset keyword for three values.
We just need to validate this order. 
It is a little bit different with two values. '50% left' or 'top 50%' are invalid and hence case 2: code is little bigger.

One doubt about Vector<RefPtr<CSSPrimitiveValue> > values, is values.clear() call required?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.