Bug 58964

Summary: Allow casting between CSSPrimitiveValue and EBorderCollapse to remove special-case logic from CSSStyleSelector.
Product: WebKit Reporter: Luke Macpherson <macpherson>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, dglazkov, eric, macpherson, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Description Luke Macpherson 2011-04-19 22:31:48 PDT
Allow casting between CSSPrimitiveValue and EBorderCollapse to remove special-case logic from CSSStyleSelector.
Comment 1 Luke Macpherson 2011-04-19 22:36:33 PDT
Created attachment 90308 [details]
Patch
Comment 2 WebKit Review Bot 2011-04-19 22:38:46 PDT
Attachment 90308 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/rendering/style/RenderStyle.h:196:  _border_collapse is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Luke Macpherson 2011-04-19 22:44:06 PDT
Style fail is due to legacy code.
Let me know what you think. Seems that there are a lot of these cases that when fixed will greatly simplify CSSStyleSelector/CSSStyleApplyProperty.
Comment 4 Eric Seidel (no email) 2011-04-19 22:55:48 PDT
Comment on attachment 90308 [details]
Patch

Sounds good to me.  style->borderColapse() isn't called anywhere?
Comment 5 Simon Fraser (smfr) 2011-04-19 22:56:14 PDT
Comment on attachment 90308 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests as no new functinoality added.

Typo
Comment 6 Luke Macpherson 2011-04-19 23:01:24 PDT
Created attachment 90309 [details]
Patch
Comment 7 WebKit Review Bot 2011-04-19 23:03:49 PDT
Attachment 90309 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/rendering/style/RenderStyle.h:196:  _border_collapse is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Luke Macpherson 2011-04-19 23:04:41 PDT
(In reply to comment #4)
> (From update of attachment 90308 [details])
> Sounds good to me.  style->borderColapse() isn't called anywhere?

The enum maps to 0,1 in the same way as the boolean.
Also the place it gets called in CSSStyleSelector (in the macro) is of the form set(get()).
Comment 9 Luke Macpherson 2011-04-19 23:04:58 PDT
(In reply to comment #5)
> (From update of attachment 90308 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90308&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        No new tests as no new functinoality added.
> 
> Typo

Fixed.
Comment 10 WebKit Commit Bot 2011-04-20 09:14:52 PDT
Comment on attachment 90309 [details]
Patch

Clearing flags on attachment: 90309

Committed r84380: <http://trac.webkit.org/changeset/84380>
Comment 11 WebKit Commit Bot 2011-04-20 09:14:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Review Bot 2011-04-20 09:28:22 PDT
http://trac.webkit.org/changeset/84380 might have broken Qt Linux Release minimal
Comment 13 Dimitri Glazkov (Google) 2011-04-20 09:47:09 PDT
(In reply to comment #12)
> http://trac.webkit.org/changeset/84380 might have broken Qt Linux Release minimal

Fix landed as http://trac.webkit.org/changeset/84385.