Bug 88804 - [CSS3 Backgrounds and Borders] Protect box-decoration-break behind a feature flag.
: [CSS3 Backgrounds and Borders] Protect box-decoration-break behind a feature ...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Alexis Menard (darktears)
:
Depends on: 88850 88861
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-11 13:59 PDT by Alexis Menard (darktears)
Modified: 2012-06-12 05:12 PDT (History)
9 users (show)

See Also:


Attachments
Patch (43.43 KB, patch)
2012-06-11 14:04 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (42.25 KB, patch)
2012-06-11 16:33 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2012-06-11 13:59:55 PDT
[CSS3 Backgrounds and Borders] Protect box-decoration-break behind a feature flag.
Comment 1 Alexis Menard (darktears) 2012-06-11 14:04:51 PDT
Created attachment 146902 [details]
Patch
Comment 2 WebKit Review Bot 2012-06-11 14:08:12 PDT
Attachment 146902 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1
Source/WebCore/rendering/style/StyleBoxData.cpp:77:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alexis Menard (darktears) 2012-06-11 14:08:50 PDT
(In reply to comment #2)
> Attachment 146902 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1
> Source/WebCore/rendering/style/StyleBoxData.cpp:77:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
> Total errors found: 1 in 27 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Please ignore, we use that trick in some other files, e.g RenderStyle.cpp.
Comment 4 Tony Chang 2012-06-11 16:28:58 PDT
Comment on attachment 146902 [details]
Patch

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

> Source/WebCore/rendering/style/RenderStyle.h:1592
> +#if ENABLE(CSS_BOX_DECORATION_BREAK)
>      static EBoxDecorationBreak initialBoxDecorationBreak() { return DSLICE; }
> +#endif

Nit: It's OK to not guard this in an ENABLE (will get compiled out if not used).

> Source/WebCore/rendering/style/RenderStyleConstants.h:106
> +#if ENABLE(CSS_BOX_DECORATION_BREAK)

Nit: It's OK to not guard this in an ENABLE (doesn't take up space).

> Source/WebCore/rendering/style/StyleBoxData.h:84
>      bool m_hasAutoZIndex : 1;
>      unsigned m_boxSizing : 1; // EBoxSizing

Not related to your change, but we should change m_hasAutoZIndex to unsigned to fix the packing on Windows.
Comment 5 Alexis Menard (darktears) 2012-06-11 16:33:19 PDT
Created attachment 146959 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-06-11 16:38:04 PDT
Attachment 146959 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1
Source/WebCore/rendering/style/StyleBoxData.cpp:77:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Alexis Menard (darktears) 2012-06-11 16:44:30 PDT
(In reply to comment #4)
> (From update of attachment 146902 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146902&action=review
> 
> > Source/WebCore/rendering/style/RenderStyle.h:1592
> > +#if ENABLE(CSS_BOX_DECORATION_BREAK)
> >      static EBoxDecorationBreak initialBoxDecorationBreak() { return DSLICE; }
> > +#endif
> 
> Nit: It's OK to not guard this in an ENABLE (will get compiled out if not used).
> 
> > Source/WebCore/rendering/style/RenderStyleConstants.h:106
> > +#if ENABLE(CSS_BOX_DECORATION_BREAK)
> 
> Nit: It's OK to not guard this in an ENABLE (doesn't take up space).
> 
> > Source/WebCore/rendering/style/StyleBoxData.h:84
> >      bool m_hasAutoZIndex : 1;
> >      unsigned m_boxSizing : 1; // EBoxSizing
> 
> Not related to your change, but we should change m_hasAutoZIndex to unsigned to fix the packing on Windows.

I'll make a patch for that separately :)
Comment 8 WebKit Review Bot 2012-06-11 19:21:58 PDT
Comment on attachment 146959 [details]
Patch for landing

Clearing flags on attachment: 146959

Committed r120029: <http://trac.webkit.org/changeset/120029>
Comment 9 WebKit Review Bot 2012-06-11 19:22:03 PDT
All reviewed patches have been landed.  Closing bug.