Bug 149901 - REGRESSION(r184895): border-image should always slice the SVG image to nine pieces when drawing it in the container element
Summary: REGRESSION(r184895): border-image should always slice the SVG image to nine p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2015-10-07 15:46 PDT by Said Abou-Hallawa
Modified: 2015-10-12 12:51 PDT (History)
6 users (show)

See Also:


Attachments
Patch (25.20 KB, patch)
2015-10-07 16:13 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
test case (1.15 KB, text/html)
2015-10-08 10:18 PDT, Said Abou-Hallawa
no flags Details
Patch (29.21 KB, patch)
2015-10-12 10:37 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mavericks (732.07 KB, application/zip)
2015-10-12 11:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-mavericks-wk2 (793.08 KB, application/zip)
2015-10-12 11:10 PDT, Build Bot
no flags Details
Patch (29.42 KB, patch)
2015-10-12 11:21 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2015-10-07 15:46:52 PDT
This is a regression of https://bugs.webkit.org/show_bug.cgi?id=139405. The consensus in the last w3 csswg meeting was to follow the current specs for the non-intrisic SVG image case. Although the nine-piece algorithm for the non-intrisic SVG is not guaranteed to be meaningful in all cases, it is working as expected in general.
Comment 1 Said Abou-Hallawa 2015-10-07 15:47:40 PDT
<rdar://problem/21995596>
Comment 2 Said Abou-Hallawa 2015-10-07 16:13:44 PDT
Created attachment 262652 [details]
Patch
Comment 3 Darin Adler 2015-10-08 09:34:35 PDT
Comment on attachment 262652 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        The nine-pieces algorithm should be applied to the border-image regardless
> +        whether the image has an intrinsic size or not. It is not guaranteed to have
> +        a meaningful border-image in all the cases of non-intrinsic size images. But
> +        it should work as expected in most of the cases. 

I am a little bit concerned about “should work as expected in most of the cases”. Is there consensus with other implementors on how this should work?

Is the test coverage sufficient?
Comment 4 Said Abou-Hallawa 2015-10-08 10:17:12 PDT
Comment on attachment 262652 [details]
Patch

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

>> Source/WebCore/ChangeLog:10
>> +        it should work as expected in most of the cases. 
> 
> I am a little bit concerned about “should work as expected in most of the cases”. Is there consensus with other implementors on how this should work?
> 
> Is the test coverage sufficient?

This link https://lists.w3.org/Archives/Public/www-style/2015Aug/0170.html is part of the discussion about this issue on the www-style list:. The issue was also discussed in the last w3 css meeting https://wiki.csswg.org/planning/paris-2015. I also discussed it on webkit with David Vest https://bugs.webkit.org/show_bug.cgi?id=139405. And it seems the consensus is to follow the specs even with its unclarity with the case where the SVG itself has no intrinsic size but the elements inside it are fixed size. FireFox is still behaving the same as the shipped Safari; I am not sure about their plan.  Chrome is behaving the same as WebKit with this patch. I attached a test case to this bug.

I will add more tests for the case where the border-image show meaningful results.
Comment 5 Said Abou-Hallawa 2015-10-08 10:18:38 PDT
Created attachment 262699 [details]
test case
Comment 6 Said Abou-Hallawa 2015-10-08 10:20:28 PDT
Actually you can try the test case which is attached to https://bugs.webkit.org/show_bug.cgi?id=139405
Comment 7 Said Abou-Hallawa 2015-10-12 10:37:20 PDT
Created attachment 262898 [details]
Patch
Comment 8 Said Abou-Hallawa 2015-10-12 10:43:12 PDT
New test cases for the relative lengths cases were added. The intrinsic SVG with fixed lengths and the non-intrinsic SVG with relative lengths are the scenarios we care about. The non-intrinsic SVG with fixed lengths is not expected to show border-image correctly in all cases.
Comment 9 Build Bot 2015-10-12 11:10:07 PDT
Comment on attachment 262898 [details]
Patch

Attachment 262898 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/272046

New failing tests:
fast/borders/border-image-fill-no-intrinsic-size.html
Comment 10 Build Bot 2015-10-12 11:10:11 PDT
Created attachment 262903 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 11 Build Bot 2015-10-12 11:10:31 PDT
Comment on attachment 262898 [details]
Patch

Attachment 262898 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/272054

New failing tests:
fast/borders/border-image-fill-no-intrinsic-size.html
Comment 12 Build Bot 2015-10-12 11:10:35 PDT
Created attachment 262904 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 13 Said Abou-Hallawa 2015-10-12 11:21:27 PDT
Created attachment 262907 [details]
Patch
Comment 14 Said Abou-Hallawa 2015-10-12 11:22:39 PDT
Re-uploaded an new patch because a file was missing.
Comment 15 WebKit Commit Bot 2015-10-12 12:51:46 PDT
Comment on attachment 262907 [details]
Patch

Clearing flags on attachment: 262907

Committed r190883: <http://trac.webkit.org/changeset/190883>
Comment 16 WebKit Commit Bot 2015-10-12 12:51:50 PDT
All reviewed patches have been landed.  Closing bug.