RESOLVED FIXED 89421
[CSSRegions]Change WEBKIT_REGION_RULE value to 16
https://bugs.webkit.org/show_bug.cgi?id=89421
Summary [CSSRegions]Change WEBKIT_REGION_RULE value to 16
Mihnea Ovidenie
Reported 2012-06-18 22:26:05 PDT
Attachments
patch (5.04 KB, patch)
2012-06-19 07:01 PDT, Andrei Onea
tony: review-
tony: commit-queue-
patch (6.47 KB, patch)
2012-06-20 07:44 PDT, Andrei Onea
no flags
patch (6.50 KB, patch)
2012-06-20 07:57 PDT, Andrei Onea
tony: review-
buildbot: commit-queue-
patch (6.53 KB, patch)
2012-06-21 01:11 PDT, Andrei Onea
no flags
Andrei Onea
Comment 1 2012-06-19 07:01:46 PDT
Tony Chang
Comment 2 2012-06-19 10:04:32 PDT
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.
Andrei Onea
Comment 3 2012-06-20 05:10:09 PDT
Thanks for your review! I'll resubmit the patch with your suggested modifications.
Andrei Onea
Comment 4 2012-06-20 07:44:04 PDT
WebKit Review Bot
Comment 5 2012-06-20 07:46:31 PDT
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.
Andrei Onea
Comment 6 2012-06-20 07:57:15 PDT
Build Bot
Comment 7 2012-06-20 08:53:31 PDT
Gyuyoung Kim
Comment 8 2012-06-20 11:31:05 PDT
Tony Chang
Comment 9 2012-06-20 11:33:28 PDT
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.
Andrei Onea
Comment 10 2012-06-21 01:11:51 PDT
WebKit Review Bot
Comment 11 2012-06-21 10:54:13 PDT
Comment on attachment 148743 [details] patch Clearing flags on attachment: 148743 Committed r120943: <http://trac.webkit.org/changeset/120943>
WebKit Review Bot
Comment 12 2012-06-21 10:54:29 PDT
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.