453641 - -moz-box-shadow does not work on tables

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Rodrigo Ruiz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

As can be seen in the URL, the -moz-box-shadow CSS style is correctly applied to divs, but not to tables.

Reproducible: Always

Steps to Reproduce:
1. Create a page containing a table
2. Apply the following style:
    -moz-box-shadow: 2px 2px 2px black;
3. Browse the page
Actual Results:  
The table does not show any shadow.

Expected Results:  
A blured shadow should appear behind the table.

Comment 1

[Rodrigo Ruiz]

I forgot to mention that the bug appears in Firefox 3.1 alpha which, if I am not wrong, uses Core 1.9. I have changed the bug meta-data accordingly.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug probably should not have been confirmed without a testcase attached to the bug.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: simplified testcase

David, you are too fast. I should have mentioned to attach a testcase while confirming the bug, sorry.

Comment 4

[Michael Ventnor]

Shadows are drawn as part of the border display list for any element type that can have a border or background. If I interpret this part of the spec right: "shadows are only drawn where borders would also be drawn" then if tables are also unresponsive to border rules then this is expected.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Testcase 2: table with border but no box shadow

Updated testcase to have a table with a red dotted border but no box shadow.

Comment 6

[Rodrigo Ruiz]

Attachment: Testcase for comparing shadows in divs and tables

I disagree. Tables CAN have borders and backgrounds, so shadows should be drawn, no matter what kind of border they have. AFAIK, tables are not unresponsive to border rules, what do you mean?

I have uploaded a new testcase to show the differences in rendering divs and tables, with and without borders.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Tables do draw their backgrounds through a different mechanism (which combines table, row, column, etc. backgrounds correctly).  Perhaps box-shadow drawing just needs to be hooked in to that?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Right.

Comment 9

[Michael Ventnor]

Attachment: Patch

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Move nsTableCellFrame::PaintCellBoxShadow into PaintTableCellBoxShadow, you don't need both.

TableBackgroundPainter::PaintCell is called for each cell. Your code is going to paint the row/column/rowgroup/columngroup shadow once for each cell in the row/column/rowgroup/columngrup. This is wrong when the shadow is partially transparent (including blurs). To fix that you should clip the shadow to mCellRect in each case --- you might want to use a helper function for that.

Your testcase should test shadows on each possible table part --- table, rowgroup, columngroup, row, column, and cell, and test with two cells to catch the double-painting bug.

Comment 11

[Michael Ventnor]

Attachment: Patch 2

That's not necessary now, I found out a place to add the display list item that all table frame types except cells call.

I'll make reftests soon, I just want to know if this code is suitable.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This approach might be OK, but I don't know how box-shadows should work with cellspacing/cellpadding and collapsed borders. Any ideas David/Bernd?

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 350903 [details] [diff] [review]
Patch 2

Alright, let's go with this. We should resolve the question of how row/rowgroup/col/colgroup shadows are rendered, and fix it in a followup change if necessary.

Comment 14

[Michael Ventnor]

Attachment: Reftests

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed to trunk as ddb79c1c4be7 and e300dc199459.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 350903 [details] [diff] [review]
Patch 2

I'd like this for 1.9.1; it's simple and it fixes a bug in a new feature.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 350903 [details] [diff] [review]
Patch 2

a191=beltzner, please make sure that we land both the patch and the reftests (thanks for those!)

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed f452da3845c3 and 50d5b4ae6a53 to 1.9.1

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: div vs. table

Michael or Roc, one thing I can see when looking closely at the attachment 337022 [details]. Why there is an extra line between the border and shadow of the div example? If you don't have a border the table example looks like blurred in comparison to the div one. Do we have still problems with divs and shadow or behave the table incorrectly? If its the former one, shall I file a new bug on that?

Comment 20

[philippe (part-time)]

(In reply to comment #19)
> Created an attachment (id=352903) [details]
> div vs. table
> 
> Michael or Roc, one thing I can see when looking closely at the attachment
> 337022 [details]. Why there is an extra line between the border and shadow of the div

That looks like bug 445741. And you're right, it happens much less with a <table> (even though I haven't tested as extensively with <table> than with <div>).

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yeah. Lets move over there.

Further I don't know if it is related to this bug but has anyone used Firebug with a current build on attachment 337022 [details]? Try to inspect the document, especially the div and table elements. You will see that the output is completely garbled.

Comment 22

[John J. Barton]

(In reply to comment #21)
> Further I don't know if it is related to this bug but has anyone used Firebug
> with a current build on attachment 337022 [details]? Try to inspect the document,
> especially the div and table elements. You will see that the output is
> completely garbled.

With FF3.0.4 and Firebug 1.4 on that page I see Firebug frames that don't match the elements. If that is what you mean by garbled,  then that is 356665, a long standing bug high on Firebug wanted list.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks John.

Otherwise I can verify the patch on trunk and 1.9.1 with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090202 Minefield/3.2a1pre ID:20090202020617

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090202 Minefield/3.2a1pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090202 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090202020439

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090202 Shiretoko/3.1b3pre