Bug 74743 - getComputedStyle for border-bottom, border-top, border-left, border-right is not implemented.
Summary: getComputedStyle for border-bottom, border-top, border-left, border-right is ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexis Menard (darktears)
URL:
Keywords:
Depends on:
Blocks: 13658 75312 75319
  Show dependency treegraph
 
Reported: 2011-12-16 13:30 PST by Alexis Menard (darktears)
Modified: 2011-12-28 12:14 PST (History)
4 users (show)

See Also:


Attachments
Patch (31.95 KB, patch)
2011-12-16 13:31 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (22.85 KB, patch)
2011-12-28 05:11 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (22.54 KB, patch)
2011-12-28 09:50 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2011-12-16 13:30:06 PST
getComputedStyle for border-bottom, border-top, border-left, border-right is not implemented.
Comment 1 Alexis Menard (darktears) 2011-12-16 13:31:18 PST
Created attachment 119662 [details]
Patch
Comment 2 Tony Chang 2011-12-16 14:45:49 PST
Comment on attachment 119662 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119662&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2155
> +            const int properties[3] = { CSSPropertyBorderBottomWidth, CSSPropertyBorderBottomStyle,
> +                                        CSSPropertyBorderBottomColor};

Nit: Space before } and the 4 other cases below.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2156
> +            return getCSSPropertyValuesForShorthandProperties(properties, 3, style.get(), cssValuePool);

I wonder if wtf/dtoa/utils.h should expose ARRAY_SIZE.  Would be a separate patch.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2407
> +        if (!isPropertyImplicit(properties[i])) {

What does this check do?

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-shorthand.html:31
> +e.style.borderBottom="20em solid blue";

Can we test the different properties using a loop instead of copy/pasting all the test cases?
Comment 3 Tony Chang 2011-12-16 14:53:39 PST
Comment on attachment 119662 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119662&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2408
> +            RefPtr<CSSValue> value = getPropertyCSSValue(properties[i]);

It looks like this causes us to update layout for each property.  Maybe it would be better to pass DoNotUpdateLayout explicitly?  Alternately, maybe move the switch statement out into a new function so you also don't have to re-calculate |style|.
Comment 4 Darin Adler 2011-12-16 16:26:29 PST
Comment on attachment 119662 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119662&action=review

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2156
>> +            return getCSSPropertyValuesForShorthandProperties(properties, 3, style.get(), cssValuePool);
> 
> I wonder if wtf/dtoa/utils.h should expose ARRAY_SIZE.  Would be a separate patch.

WTF_ARRAY_LENGTH is in wtf/StdLibExtras.h
Comment 5 Alexis Menard (darktears) 2011-12-28 05:11:08 PST
Created attachment 120654 [details]
Patch
Comment 6 Tony Chang 2011-12-28 09:43:54 PST
Comment on attachment 120654 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120654&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2144
> +            const int properties[3] = { CSSPropertyBorderBottomWidth, CSSPropertyBorderBottomStyle,
> +                                        CSSPropertyBorderBottomColor};

Nit: Space before }

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2151
> +            const int properties[3] = { CSSPropertyBorderLeftWidth, CSSPropertyBorderLeftStyle,
> +                                        CSSPropertyBorderLeftColor};

Nit: Space before }

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2159
> +            const int properties[3] = { CSSPropertyBorderRightWidth, CSSPropertyBorderRightStyle,
> +                                        CSSPropertyBorderRightColor};

Nit: Space before }

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2166
> +            const int properties[3] = { CSSPropertyBorderTopWidth, CSSPropertyBorderTopStyle,
> +                                        CSSPropertyBorderTopColor};

Nit: Space before }
Comment 7 Alexis Menard (darktears) 2011-12-28 09:50:40 PST
Created attachment 120683 [details]
Patch for landing
Comment 8 WebKit Review Bot 2011-12-28 12:14:02 PST
Comment on attachment 120683 [details]
Patch for landing

Clearing flags on attachment: 120683

Committed r103774: <http://trac.webkit.org/changeset/103774>
Comment 9 WebKit Review Bot 2011-12-28 12:14:06 PST
All reviewed patches have been landed.  Closing bug.