Closed Bug 453641 Opened 16 years ago Closed 16 years ago

-moz-box-shadow does not work on tables

Categories

(Core :: Layout: Tables, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: rruiz, Assigned: ventnor.bugzilla)

References

()

Details

(Keywords: css3, testcase, verified1.9.1)

Attachments

(5 files, 2 obsolete files)

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.
Component: Layout: Tables → Style System (CSS)
Keywords: css3
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → 1.9.0 Branch
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.
Blocks: 212633
Version: 1.9.0 Branch → Trunk
Summary: -moz-box-shadow applies to divs, but not to tables → -moz-box-shadow does not work on tables
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: layout.tables → style-system
Component: Style System (CSS) → Layout: Tables
QA Contact: style-system → layout.tables
This bug probably should not have been confirmed without a testcase attached to the bug.
Attached file simplified testcase (obsolete) —
David, you are too fast. I should have mentioned to attach a testcase while confirming the bug, sorry.
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.
Updated testcase to have a table with a red dotted border but no box shadow.
Attachment #336900 - Attachment is obsolete: true
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.
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?
Keywords: testcase
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → ventnor.bugzilla
Attachment #349838 - Flags: superreview?(roc)
Attachment #349838 - Flags: review?(roc)
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.
Attached patch Patch 2Splinter Review
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.
Attachment #349838 - Attachment is obsolete: true
Attachment #350903 - Flags: superreview?(roc)
Attachment #350903 - Flags: review?(roc)
Attachment #349838 - Flags: superreview?(roc)
Attachment #349838 - Flags: review?(roc)
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?
Attachment #350903 - Flags: superreview?(roc)
Attachment #350903 - Flags: superreview+
Attachment #350903 - Flags: review?(roc)
Attachment #350903 - Flags: review+
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.
Attached patch ReftestsSplinter Review
Keywords: checkin-needed
Whiteboard: [needs landing]
Pushed to trunk as ddb79c1c4be7 and e300dc199459.
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 approval]
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.
Attachment #350903 - Flags: approval1.9.1?
Attachment #350903 - Flags: approval1.9.1? → approval1.9.1+
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!)
Whiteboard: [needs 191 approval] → [needs 191 landing]
Pushed f452da3845c3 and 50d5b4ae6a53 to 1.9.1
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
Target Milestone: --- → mozilla1.9.1b3
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1
Attached image 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?
(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>).
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.
(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.
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: