[webkit-reviews] review requested: [Bug 9284] Quirksmode (CSS1): Removing inline border styles is impossible : [Attachment 12303] Expand shorthands for removal

[bugzilla-request-daemon at macosforge.org]

mitz at webkit.org has asked  for review:
Bug 9284: Quirksmode (CSS1): Removing inline border styles is impossible
http://bugs.webkit.org/show_bug.cgi?id=9284

Attachment 12303: Expand shorthands for removal
http://bugs.webkit.org/attachment.cgi?id=12303&action=edit

------- Additional Comments from mitz at webkit.org
(In reply to comment #7)
> +struct PropertyLonghand
> +{
> 
> We should put the brace on the line iwth the struct name.

Done.

> +static HashMap<int, PropertyLonghand* >* shorthandMap;
> 
> I don't think the space before the > is needed here. I think the values
should
> be PropertyLonghand, no need for pointers.

Done.

> The shortHandMap could be a static inside the
> CSSMutableStyleDeclaration::removeProperty function -- I think I like that
> slightly better.

Done.

> It would be nice if the function that turns a propertyID into a pointer to a
> list of property IDs and a length was put with the other shorthand handling
> rather than right here in CSSMutableStyleDeclaration --
> CSSMutableStyleDeclaration could just call a function defined there. That
might
> make it easier to keep the list of shorthands in sync with the other lists of

> shorthands.

Agreed, but I think that can (and should) be a part of bug 12159.

> That technique is not reliable. It relies on the four arrays being
consecutive
> in global data, something that is not guaranteed by the C++ language.

Fixed.

I also updated the set of properties in this patch to be exactly those
properties
that are parsed as shorthands. Interestingly enough (for me) the list doesn't
include
'font', '-webkit-border-radius' and 'overflow'.

Added change log and a test.