Bug 28017 – box-shadow's spread is ignored with <table>

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

[Shinichiro Hamaji]

Created attachment 34132 [details]
Test case

The change for Bug 27811 is needed for RenderTable as well. See the test case. I'll send a patch soon later.

[Shinichiro Hamaji]

Created attachment 34133 [details]
Patch v1


---
 6 files changed, 164 insertions(+), 7 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 34133 [details]
Patch v1

If you're going to edit the generated template like this, you need to also modify make-js-test-wrappers so ignore your .js file so that it doens't try to re-gen a new version every time someone else runs make-js-test-wrappers.  (Yes, I know this is all very poorly documented.)

Seems silly to leave this in the output:
 You should see no red.
 16 div:
 17 
 18 flexbox:
 19 
 20 table:
 21 

If you wrap it all in a div, you can easily remove that div before ending your script.

Otherwise the change looks fine to me.

r- for those nits.

[Shinichiro Hamaji]

Created attachment 34237 [details]
Patch v2


---
 8 files changed, 160 insertions(+), 7 deletions(-)

[Shinichiro Hamaji]

Created attachment 34239 [details]
Patch v3


---
 8 files changed, 168 insertions(+), 7 deletions(-)

[Shinichiro Hamaji]

Thanks for the quick review!

I modified my patch as you commented (please just ignore Patch v2). I also modified a few things:

- Change the filename of layout test. resources/overflow-scroll.js may be too general to put it into make-js-test-wrappers.
- Make the color of shadow green. People may see the test is passing easily.

[Eric Seidel (no email)]

Comment on attachment 34239 [details]
Patch v3

LGTM.

[Shinichiro Hamaji]

Created attachment 34245 [details]
Patch v4


---
 8 files changed, 168 insertions(+), 7 deletions(-)

[Shinichiro Hamaji]

Oops, I used wrong email addresses... Hmm I should check why prepare-ChangeLog obtained the addresses.

[Adam Barth]

Comment on attachment 34245 [details]
Patch v4

Presumably this patch is still ok if you only changed your email address.

[Adam Barth]

Comment on attachment 34245 [details]
Patch v4

Clearing review flag on attachment: 34245

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/box-shadow/box-shadow-overflow-scroll-expected.txt
	A	LayoutTests/fast/box-shadow/box-shadow-overflow-scroll.html
	A	LayoutTests/fast/box-shadow/resources/box-shadow-overflow-scroll.js
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderTable.cpp
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/make-js-test-wrappers
Committed r46888
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderTable.cpp
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/box-shadow/box-shadow-overflow-scroll-expected.txt
	A	LayoutTests/fast/box-shadow/resources/box-shadow-overflow-scroll.js
	A	LayoutTests/fast/box-shadow/box-shadow-overflow-scroll.html
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/make-js-test-wrappers
r46888 = a9f708a39f4c71132e92f7d9ccb0b1d36fe62e49 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46888

[Adam Barth]

All reviewed patches have been landed.  Closing bug.