Bug 75319 - getComputedStyle for border is not implemented.
: getComputedStyle for border is not implemented.
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Alexis Menard (darktears)
:
Depends on: 74743
Blocks: 13658
  Show dependency treegraph
 
Reported: 2011-12-28 12:07 PST by Alexis Menard (darktears)
Modified: 2012-01-03 11:14 PST (History)
6 users (show)

See Also:


Attachments
Patch (18.54 KB, patch)
2011-12-28 12:09 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (19.79 KB, patch)
2011-12-29 03:30 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (6.76 KB, patch)
2012-01-02 03:43 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-28 12:07:53 PST
getComputedStyle for border is not implemented.
Comment 1 Alexis Menard (darktears) 2011-12-28 12:09:12 PST
Created attachment 120689 [details]
Patch
Comment 2 Tony Chang 2011-12-28 13:59:12 PST
Comment on attachment 120689 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2146
> +            list->append(getPropertyCSSValue(CSSPropertyBorderTop, DoNotUpdateLayout));
> +            list->append(getPropertyCSSValue(CSSPropertyBorderRight, DoNotUpdateLayout));
> +            list->append(getPropertyCSSValue(CSSPropertyBorderBottom, DoNotUpdateLayout));
> +            list->append(getPropertyCSSValue(CSSPropertyBorderLeft, DoNotUpdateLayout));

Can you use getCSSPropertyValuesForShorthandProperties here?
Comment 3 Tony Chang 2011-12-28 14:00:39 PST
Comment on attachment 120689 [details]
Patch

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

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2146
>> +            list->append(getPropertyCSSValue(CSSPropertyBorderLeft, DoNotUpdateLayout));
> 
> Can you use getCSSPropertyValuesForShorthandProperties here?

Oh, is it because this is comma separated?  Can we add a param to getCSSPropertyValuesForShorthandProperties for this?
Comment 4 Alexis Menard (darktears) 2011-12-29 03:30:27 PST
Created attachment 120729 [details]
Patch
Comment 5 Tony Chang 2011-12-29 09:44:13 PST
Comment on attachment 120729 [details]
Patch

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

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-shorthand-expected.txt:116
> +PASS computedStyle.getPropertyCSSValue('border').cssText is '320px solid rgb(255, 0, 0), 320px solid rgb(255, 0, 0), 320px solid rgb(255, 0, 0), 320px solid rgb(255, 0, 0)'

This doesn't parse when assigned to border. I think it's important that the value returned by getComputedStyle is a valid CSS value.

There are some values where border can't represent the border state (when they are different for each side).  I think we have 2 options:
1) Never return a value for border (what we do now).
2) Check to see if the four borders are the same and return the computed style if they are all the same.  Otherwise return nothing.
Comment 6 Alexis Menard (darktears) 2011-12-29 09:57:03 PST
(In reply to comment #5)
> (From update of attachment 120729 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120729&action=review
> 
> > LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-shorthand-expected.txt:116
> > +PASS computedStyle.getPropertyCSSValue('border').cssText is '320px solid rgb(255, 0, 0), 320px solid rgb(255, 0, 0), 320px solid rgb(255, 0, 0), 320px solid rgb(255, 0, 0)'
> 
> This doesn't parse when assigned to border. I think it's important that the value returned by getComputedStyle is a valid CSS value.

This is a good point.

> 
> There are some values where border can't represent the border state (when they are different for each side).  I think we have 2 options:
> 1) Never return a value for border (what we do now).
> 2) Check to see if the four borders are the same and return the computed style if they are all the same.  Otherwise return nothing.

The second option is what Opera is doing.
Comment 7 Alexis Menard (darktears) 2012-01-02 03:43:02 PST
Created attachment 120870 [details]
Patch
Comment 8 Alexis Menard (darktears) 2012-01-02 03:44:22 PST
(In reply to comment #7)
> Created an attachment (id=120870) [details]
> Patch

Implemented as discussed in http://lists.w3.org/Archives/Public/www-style/2011Dec/0551.html

It's the option 2 that Tony proposed.
Comment 9 WebKit Review Bot 2012-01-03 11:14:16 PST
Comment on attachment 120870 [details]
Patch

Clearing flags on attachment: 120870

Committed r103948: <http://trac.webkit.org/changeset/103948>
Comment 10 WebKit Review Bot 2012-01-03 11:14:21 PST
All reviewed patches have been landed.  Closing bug.