Summary: | [CSSRegions]Change WEBKIT_REGION_RULE value to 16 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mihnea Ovidenie <mihnea> | ||||||||||
Component: | CSS | Assignee: | Mihnea Ovidenie <mihnea> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, andreionea3000, cmarcelo, macpherson, menard, ojan, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Mihnea Ovidenie
2012-06-18 22:26:05 PDT
Created attachment 148326 [details]
patch
Comment on attachment 148326 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=148326&action=review Just some minor nits to make sure we don't make mistakes in the future. > Source/WebCore/css/StyleRule.h:48 > - Region > + Region = 16 If it's important for these to be kept in sync with the values in CSSRule.h, you could use a COMPILE_ASSERT. E.g., in CSSRule.cpp, COMPILE_ASSERT(StyleRule::Region == CSSRule::WEBKIT_REGION_RULE, enums_should_match);. > Source/WebCore/css/StyleRule.h:87 > - unsigned m_type : 4; > - signed m_sourceLine : 28; > + unsigned m_type : 5; > + signed m_sourceLine : 27; Can you add a compile assert for the size in StyleRule.cpp? See CSSRule.cpp for an example. Thanks for your review! I'll resubmit the patch with your suggested modifications. Created attachment 148565 [details]
patch
Attachment 148565 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:13: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:14: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:17: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:22: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:25: Line contains tab character. [whitespace/tab] [5]
Total errors found: 6 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 148567 [details]
patch
Comment on attachment 148567 [details] patch Attachment 148567 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13002221 Comment on attachment 148567 [details] patch Attachment 148567 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13001259 Comment on attachment 148567 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=148567&action=review > Source/WebCore/css/CSSRule.cpp:49 > +COMPILE_ASSERT(StyleRuleBase::Region == static_cast<StyleRuleBase::Type>(CSSRule::WEBKIT_REGION_RULE), enums_should_match); Looks like this needs to be in an #if ENABLE(CSS_REGIONS) block. Created attachment 148743 [details]
patch
Comment on attachment 148743 [details] patch Clearing flags on attachment: 148743 Committed r120943: <http://trac.webkit.org/changeset/120943> All reviewed patches have been landed. Closing bug. |