Bug 75434 - getComputedStyle should return shorthands properties with the minimum number of sides possible.
: getComputedStyle should return shorthands properties with the minimum number ...
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 13658
  Show dependency treegraph
 
Reported: 2012-01-02 05:35 PST by
Modified: 2012-01-03 11:57 PST (History)


Attachments
Patch (34.43 KB, patch)
2012-01-02 05:38 PST, Alexis Menard (darktears)
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (34.66 KB, patch)
2012-01-03 10:35 PST, Alexis Menard (darktears)
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-01-02 05:35:18 PST
getComputedStyle should return shorthands property with the minimum number of sides possible.
------- Comment #1 From 2012-01-02 05:38:43 PST -------
Created an attachment (id=120873) [details]
Patch
------- Comment #2 From 2012-01-03 10:14:05 PST -------
(From update of attachment 120873 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=120873&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2419
> +PassRefPtr<CSSValueList> CSSComputedStyleDeclaration::getCSSProperty4ValuesForShorthandProperties(const int* properties) const
> +{
> +    RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
> +    // Assume the properties are in the usual order top, right, bottom, left.

I would try to make the name more clear that this function is for properties that apply to each direction/side.  For example, getCSSPropertyValuesForSidesShorthand or something.

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-width-expected.txt:24
> +PASS computedStyle.getPropertyCSSValue('border-width').cssText is '10px 0px 0px'

In a follow up patch, maybe we should change 0px to 0 since the unit identifier is optional on zero length units. http://www.w3.org/TR/css3-values/#length-value
------- Comment #3 From 2012-01-03 10:16:39 PST -------
(In reply to comment #2)
> (From update of attachment 120873 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120873&action=review
> 
> > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2419
> > +PassRefPtr<CSSValueList> CSSComputedStyleDeclaration::getCSSProperty4ValuesForShorthandProperties(const int* properties) const
> > +{
> > +    RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
> > +    // Assume the properties are in the usual order top, right, bottom, left.
> 
> I would try to make the name more clear that this function is for properties that apply to each direction/side.  For example, getCSSPropertyValuesForSidesShorthand or something.

Yes I ran out of imagination on that one :(. I like your suggestion.

> 
> > LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-width-expected.txt:24
> > +PASS computedStyle.getPropertyCSSValue('border-width').cssText is '10px 0px 0px'
> 
> In a follow up patch, maybe we should change 0px to 0 since the unit identifier is optional on zero length units. http://www.w3.org/TR/css3-values/#length-value

I'll look at that one.
------- Comment #4 From 2012-01-03 10:35:07 PST -------
Created an attachment (id=120957) [details]
Patch for landing
------- Comment #5 From 2012-01-03 11:57:37 PST -------
(From update of attachment 120957 [details])
Clearing flags on attachment: 120957

Committed r103953: <http://trac.webkit.org/changeset/103953>
------- Comment #6 From 2012-01-03 11:57:42 PST -------
All reviewed patches have been landed.  Closing bug.