Summary: | Allow casting between CSSPrimitiveValue and EBorderCollapse to remove special-case logic from CSSStyleSelector. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Luke Macpherson <macpherson> | ||||||
Component: | New Bugs | Assignee: | 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
Luke Macpherson
2011-04-19 22:31:48 PDT
Created attachment 90308 [details]
Patch
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.
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 on attachment 90308 [details]
Patch
Sounds good to me. style->borderColapse() isn't called anywhere?
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 Created attachment 90309 [details]
Patch
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.
(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()). (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 on attachment 90309 [details] Patch Clearing flags on attachment: 90309 Committed r84380: <http://trac.webkit.org/changeset/84380> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/84380 might have broken Qt Linux Release minimal (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. |