All users were logged out of Bugzilla on October 13th, 2018

-moz-box-shadow does not work on tables

VERIFIED FIXED in mozilla1.9.2a1

Status

()

VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: rruiz, Assigned: ventnor.bugzilla)

Tracking

({css3, testcase, verified1.9.1})

Trunk
mozilla1.9.2a1
css3, testcase, verified1.9.1
Points:
---
Bug Flags:
wanted1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Updated

10 years ago
Component: Layout: Tables → Style System (CSS)
Keywords: css3
OS: Windows XP → All
Hardware: PC → All
(Reporter)

Updated

10 years ago
Version: unspecified → 1.9.0 Branch
(Reporter)

Comment 1

10 years ago
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.
Created attachment 336900 [details]
simplified testcase

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

Comment 4

10 years ago
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.
Created attachment 336931 [details]
Testcase 2: table with border but no box shadow

Updated testcase to have a table with a red dotted border but no box shadow.
Attachment #336900 - Attachment is obsolete: true
(Reporter)

Comment 6

10 years ago
Created attachment 337022 [details]
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.
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?

Updated

10 years ago
Keywords: testcase
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+
(Assignee)

Comment 9

10 years ago
Created attachment 349838 [details] [diff] [review]
Patch
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.
(Assignee)

Comment 11

10 years ago
Created attachment 350903 [details] [diff] [review]
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.
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.
(Assignee)

Comment 14

10 years ago
Created attachment 351065 [details] [diff] [review]
Reftests
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
Pushed to trunk as ddb79c1c4be7 and e300dc199459.
Status: NEW → RESOLVED
Last Resolved: 10 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
Created attachment 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 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

10 years ago
(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.

Comment 22

10 years ago
(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
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.