Bug 67321 – [CSS3 Backgrounds and Borders] Implement border-image-slice

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Dave Hyatt]

This bug is tracking the implementation of the border-image-slice property.

[Dave Hyatt]

Created attachment 105833 [details]
Patch

[WebKit Review Bot]

Attachment 105833 [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:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebCore/css/CSSBorderImageValue.h:58:  The parameter name "slice" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/css/CSSParser.h:39:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/css/CSSParser.h:170:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSStyleSelector.cpp:5733:  l is incorrectly named. Don't use the single letter 'l' as an identifier name.  [readability/naming] [4]
Source/WebCore/css/CSSBorderImageSliceValue.h:47:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WebCore/css/CSSParser.cpp:5296:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.cpp:5360:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 8 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 105833 [details]
Patch

Attachment 105833 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9572676

[Beth Dakin]

Comment on attachment 105833 [details]
Patch

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

r=me! Dave mentioned on irc that he fixed all of the style issues except the RefPtr thing, which there is an open question about. Either way, parseBorderImage (which already exists in the codebase) does the same thing, so it's probably okay to check some more similar code in while Dave's figuring out the right thing.

> Source/WebCore/css/CSSParser.cpp:5318
> +    PassRefPtr<CSSBorderImageSliceValue> commitBorderImageSlice()

I feel like some comments would be nice in this function. I think I can make out why m_top is asserted and why m_bottom and m_left are set if there is no m_right by looking at the patch as a whole, but some comments would be nice so that it's less mysterious out of context.

[WebKit Review Bot]

Comment on attachment 105833 [details]
Patch

Attachment 105833 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9565957

New failing tests:
fast/gradients/border-image-gradient-sides-and-corners.html
fast/css/getComputedStyle/computed-style-without-renderer.html
svg/css/getComputedStyle-basic.xhtml
fast/css/getComputedStyle/computed-style.html
fast/reflections/reflection-computed-style.html
fast/borders/border-image-slices.html

[Dave Hyatt]

Created attachment 105883 [details]
New patch to test out EWS etc.

[WebKit Review Bot]

Attachment 105883 [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

Last 3072 characters of output:
ved.  [readability/parameter_name] [5]
Source/WebCore/css/CSSParser.h:148:  The parameter name "rgb" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/css/CSSParser.h:169:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:170:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:178:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:180:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:181:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:182:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:186:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:186:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:186:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:187:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:187:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:225:  The parameter name "marginBox" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/css/CSSParser.h:309:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/css/CSSParser.h:323:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/css/CSSParser.h:327:  yy_c_buf_p is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/css/CSSParser.h:328:  yy_hold_char is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/css/CSSParser.h:329:  yy_last_accepting_state is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/css/CSSParser.h:330:  yy_last_accepting_cpos is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/css/CSSParser.h:333:  yy_start is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/css/CSSParser.h:357:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/css/CSSParser.h:367:  Missing spaces around |  [whitespace/operators] [3]
Source/WebCore/css/CSSParser.cpp:5360:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 41 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dave Hyatt]

LOL so fixing CSSParser's indentation namespace problem just exposes me to a zillion style errors. I give up.

[Early Warning System Bot]

Comment on attachment 105883 [details]
New patch to test out EWS etc.

Attachment 105883 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9578199

[WebKit Review Bot]

Comment on attachment 105883 [details]
New patch to test out EWS etc.

Attachment 105883 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9578208

New failing tests:
svg/css/getComputedStyle-basic.xhtml
fast/css/getComputedStyle/computed-style.html
fast/borders/border-image-slices.html
fast/css/getComputedStyle/computed-style-without-renderer.html

[Collabora GTK+ EWS bot]

Comment on attachment 105833 [details]
Patch

Attachment 105833 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9570862

[Collabora GTK+ EWS bot]

Comment on attachment 105883 [details]
New patch to test out EWS etc.

Attachment 105883 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9570899

[Csaba Osztrogonác]

Original patch landed in http://trac.webkit.org/changeset/94299

And I had to fix the Qt build ( http://trac.webkit.org/changeset/94304 ) , because Dave was too lazy to respect WebKit policies and fix the build before and after landing ... :(

[Simon Fraser (smfr)]

This broke http://build.webkit.org/results/Lion%20Intel%20Debug%20(Tests)/r94299%20(410)/fast/dom/prototype-inheritance-2-pretty-diff.html

[Ryosuke Niwa]

According to TestFailures, this patch caused the following 4 tests to fail on Snow Leopard:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r94299%20(32757)/svg/css/getComputedStyle-basic-pretty-diff.html
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r94299%20(32757)/fast/dom/prototype-inheritance-2-pretty-diff.html
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r94299%20(32757)/fast/css/getComputedStyle/computed-style-without-renderer-pretty-diff.html
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r94299%20(32757)/fast/css/getComputedStyle/computed-style-pretty-diff.html

[Dave Hyatt]

Fixed.