Bug 48771 – Support background-clip: content-box

• Component: CSS
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

parseBackgroundClip() doesn't accept content-box as a value for background-clip, but the spec includes it.

Created attachment 78430 [details]
Patch to add content-box as a valid background-clip value

This patch adds "content-box" as a valid value for background-clip.

[Simon Fraser (smfr)]

Comment on attachment 78430 [details]
Patch to add content-box as a valid background-clip value

Please also add a pixel test in fast/css that tests whether the value actually works.

Created attachment 78843 [details]
Patch to add content-box as a valid background-clip value

Added pixel test to test the various background-clip values.

[Simon Fraser (smfr)]

Comment on attachment 78843 [details]
Patch to add content-box as a valid background-clip value

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

> LayoutTests/fast/css/background-clip-values.html:36
> +<div id="box1">
> +Background color should be clipped to the content box. Padding space is white.
> +</div>
> +<br>
> +Background color should be clipped to the padding box. Padding space is yellow.
> +<br>
> +Background color should be clipped to the border box. Yellow should be visible within the dashed border.
> +</div>
> +</body>

If you remove the visible text from the testcase, your pixel results could be cross-platform (just put them in the same directory as the tests). You can keep the text there in an HTML comment.

Created attachment 78853 [details]
Patch to add content-box as a valid background-clip value

Thanks for the review. I hope I have incorporated your suggestions correctly. Thanks.

[Simon Fraser (smfr)]

Comment on attachment 78853 [details]
Patch to add content-box as a valid background-clip value

r=me, but it would be good to make the borders thicker in your testcase so that the last two boxes are more obviously different.

Created attachment 78984 [details]
Patch to add content-box as a valid background-clip value

Made changes to suggested in the review.

[Simon Fraser (smfr)]

Comment on attachment 78984 [details]
Patch to add content-box as a valid background-clip value

Very close! But the scrollbar in the pixel results will cause problems because scrollbars look different on different platforms.

Created attachment 79199 [details]
Patch to add content-box as a valid background-clip value

Hope this be will be it!

Hi Simon,
I see a r- and do not see any comments/reason as to why it is a r- ? could you please elaborate. Thanks for taking time to review.

[Simon Fraser (smfr)]

Comment on attachment 79199 [details]
Patch to add content-box as a valid background-clip value

Sorry, I picked the wrong option! r+

Thanks a lot Simon for the review, and thanks Chang for committing the same

[WebKit Commit Bot]

Comment on attachment 79199 [details]
Patch to add content-box as a valid background-clip value

Rejecting attachment 79199 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-sf-cq', 'bu..." exit_code: 2

Last 500 characters of output:
bgl ...........................................................................
fast/clip ....................
fast/compact ...
fast/constructors .
fast/cookies .
fast/css-generated-content ...........................................
fast/css ........................
fast/css/background-clip-values.html -> failed

Exiting early after 1 failures. 6396 tests run.
144.62s total testing time

6395 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
3 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/7542173

Created attachment 79297 [details]
Patch to add content-box as a valid background-clip value

Removed the <br> from background-clip-values.html pixel test since it seems to be causing cross platform issues while running layout tests.

[Chang Shu]

Comment on attachment 79297 [details]
Patch to add content-box as a valid background-clip value

try again

[WebKit Commit Bot]

Comment on attachment 79297 [details]
Patch to add content-box as a valid background-clip value

Clearing flags on attachment: 79297

Committed r76047: <http://trac.webkit.org/changeset/76047>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Please check to see if any pixel results need to be updated after this change. I think at least fast/backgrounds/size/backgroundSize22.html needs new results.

Hi Simon,

I am working on another project, but will definitely check this when I find time.

Thanks