Bug 75319

Summary: getComputedStyle for border is not implemented.
Product: WebKit Reporter: Alexis Menard (darktears) <menard>
Component: CSSAssignee: Alexis Menard (darktears) <menard>
Status: RESOLVED FIXED    
Severity: Normal CC: kling, koivisto, macpherson, simon.fraser, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 74743    
Bug Blocks: 13658    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Alexis Menard (darktears)
Reported 2011-12-28 12:07:53 PST
getComputedStyle for border is not implemented.
Attachments
Patch (18.54 KB, patch)
2011-12-28 12:09 PST, Alexis Menard (darktears)
no flags
Patch (19.79 KB, patch)
2011-12-29 03:30 PST, Alexis Menard (darktears)
no flags
Patch (6.76 KB, patch)
2012-01-02 03:43 PST, Alexis Menard (darktears)
no flags
Alexis Menard (darktears)
Comment 1 2011-12-28 12:09:12 PST
Tony Chang
Comment 2 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?
Tony Chang
Comment 3 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?
Alexis Menard (darktears)
Comment 4 2011-12-29 03:30:27 PST
Tony Chang
Comment 5 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.
Alexis Menard (darktears)
Comment 6 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.
Alexis Menard (darktears)
Comment 7 2012-01-02 03:43:02 PST
Alexis Menard (darktears)
Comment 8 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.
WebKit Review Bot
Comment 9 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>
WebKit Review Bot
Comment 10 2012-01-03 11:14:21 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.