Background Images on tables are not showing up in Quirks mode

VERIFIED FIXED in M16

Status

()

Core
Layout: Tables
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: Marc Attinasi, Assigned: Marc Attinasi)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
This seems to be a recent regression because it used to work fine. If you are in 
Quirks mode and a table has a background image, then the image will not be 
rendered. In Standard mode it works correctly. Verified on NT, Win95 and Linux 
and first noticed on 3/31 (see bug 30024 for comments about when this was 
noticed)

Comment 1

18 years ago
Don, can you take a look, since you have modified background code recently.
Assignee: karnaze → dcone

Comment 2

18 years ago
From what I can tell, the Paintbackground is being called.. but the 
aColor(nsStyleColor) paramater being passed in (which has the background image 
in it) is empty.
Assignee: dcone → karnaze
(Assignee)

Comment 3

18 years ago
nsTableFrame::Paint has the following code that only paints the table 
background in standard mode:

      if (eCompatibility_Standard == mode) {
        nsCSSRendering::PaintBackground(aPresContext, aRenderingContext, this,
                                        aDirtyRect, rect, *color, *spacing, 0, 

I believe it used to inherit the image into the cells and then paint the 
background for the cells (that is how Nav4 does it, which is clearly wrong), but 
that inheritance is not happening now so we get nothing...
(Assignee)

Comment 4

18 years ago
Thinking about it again from the style side, we never inherit a background image 
from a table into a table cell (we never inherit background image at all). There 
must have been something in the TableCellFrame doing that quirk behavior. 

I think it would be good to just render the background image in the table in 
Quirk mode, especially since that is how IE does it. Nav4's way of doing table 
background images is just plain wrong (repeating the image in each cell).

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → M17
(Assignee)

Comment 5

18 years ago
*** Bug 36461 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 6

18 years ago
*** Bug 36575 has been marked as a duplicate of this bug. ***

Comment 7

18 years ago
Mark, reassigning as we discussed.
Assignee: karnaze → attinasi
Status: ASSIGNED → NEW
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: M17 → M16
(Assignee)

Comment 8

18 years ago
Fixed. We now draw table backgrounds in both NavQuirks and Standard mode. 

Note, however, that the bogus way that Nav did background images is no longer 
emulated; we always draw the background image for the whole table, not repeated 
for each cell as Nav did it.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 9

18 years ago
I submitted a bug report for one of my web sites that is also having this very
same problem.  The bug 36461 was marked as a duplicate of this one.  This
problem still exists, and can be seen at the following page.

http://www.testequity.com/nav1.phtml

The navigation bar at the top of the page should have a ridged graphic going all
the way across.  This used to work just fine with Mozilla about a month back,
but has been broken for a while now.  As of build 2000050215 this can still be seen.

Reopening bug, and adding myself to cc: list.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

18 years ago
What happens in NavQuirks mode (the mode your page uses since it has the 
TRANSITIONAL DTD) is that the background color will be drawn over the entire 
table, then the background image will be drawn over the background color, but 
not in the cells. If you had any padding set you would see the background image 
in the padding area. Also, if you remove the background color you will see the 
background image over the entire table, including in the cells.

In standard mode, of course, this is all as it is uppossed to be. We still have 
problems getting the table background right, so I'll accept this and look into 
it further.

Thanks Michael.
Status: REOPENED → ASSIGNED
(Assignee)

Comment 11

18 years ago
After some more investigation I have found that the background color is being 
inherited into the cells from the table in NavQuirks mode by the 
TableBackgroundRule. This is not correct, I don't know why it was in there... 
I think it was because the Quirk-only rule was trying to emulate Nav4 
by propagating the background image and color, and we removed the 
background image inheritance some time ago, leaving the color to inherit. 

Removing the propogation of the background color from table to cell fixes the 
problem. I need to test some other pages and do a little digging around to find 
out why it was doing that in the first place. This really means the entire 
TableBackgroundRule can now be removed, if it pans-out.

Comment 12

18 years ago
Marc,

Thank you for looking into this.  I just pulled in build 2000050308 and as I'm
sure you would expect, the problem we're discussing is still there.  What is
different now is that the front page is all whacked out.

http://www.testequity.com/

This must have JUST gotten into the tree, since I was just looking at this with
last night's build and there weren't any probs.  This may be needing a seperate
bug report, but seeing as how you're doing a lot of work with table rules at the
moment I thought I'd mention it here before posting a new bug.  If you think I
should post a seperate report, please let me know.
(Assignee)

Comment 13

18 years ago
The background color and images now should work correctly in NavQuirks mode - I 
put back in the code that was making it work a month or so ago and that I had 
mistakenly #ifdef'd out.

nsHTMLStyleSheet.cpp, nsTableFrame.cpp were changed

Marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

18 years ago
Michael, regarding the new problems on http://www.testequity.com :

This may be related to recent table reflow changes that were landed in the last 
week. Can you see if there is a bug already on it, maybe reduce the page to a 
testcase? If you cannot, then I would just open a new bug and let somebody else 
do the triage. It surely looks like the table layout problem. Karnaze is already 
on the CC list so maybe he can look at it too?

Thanks for your help on the table background problem.

Comment 15

18 years ago
i've no testcase yet, but i've made a few backtraces in gdb after stopping
mozilla while the cpu was at 100%. hope this helps...

Comment 16

18 years ago
ooops... sorry, wrong bug number. my last comment was intended to go with
http://bugzilla.mozilla.org/show_bug.cgi?id=38033

Comment 17

18 years ago
Marc,

I not only concur with this bug being moved to fixed, things are really looking
a lot better than in previous builds.  The overall rendering is a lot snappier,
and things are appearing exactly where I would expect them to.

I'm not going to be posting a bug report on the front page table problems,
because they now appear to be fixed in this latest build.  Fixed, and again
rendering much nicer.  The reflow is going what appears to be x10 faster than in
any of the previous builds.

Heck, while I'm on my rant here, I've even noticed a couple of other long term
bugs get fixed as it relates to this site.  Outstanding work!

Comment 18

18 years ago
see bug 38388 (about http://engsoc.queensu.ca/sci00/ and red stripes not
repeating till a refresh is forced. Is it a dup of this or something new?
I can't reproduce it but there's still trouble.
And the reflow there is horked in 2000-050609:
When scrolling up/down the images on top refresh wrong so you see partly two of
them for a while. (the whole images and then half of them on top)

Comment 19

18 years ago
I think it's a different issue: the image tiling code. See my comments on bug 
38388.

Comment 20

18 years ago
Running build 2000050513 and I'm not seeing that tiling problem at all.  I'll
pull down a later build and doublecheck.  This must have just hit the tree.

Comment 21

18 years ago
Confirming fixed on Linux build ID 2000050709 and Windows build ID 2000050708.
Status: RESOLVED → VERIFIED

Comment 22

18 years ago
The table background image repeat problem was fixed in a previous build, 
however, I downloaded build 2000050820, and the issue is still there.
http://contenttracker.atlascomm.net 
The table should show an image of a crosshair in the background, however, it 
repeats 3 times (once in each new row).
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 23

18 years ago
Having the table background image repeat for each cell is the correct NavQuirk 
behavior. I did make a change that caused that behavior to go away, however I 
had to put it back because it broke too many pages. Instead, I made another 
change to allow the author to override the table background image inheritance 
using background="" on the TD - this is a common technique used to limit the 
quirky navigator behavior. For your page, you can make the table look the same 
for Nav, Mozilla and IE by putting the following in each TD you do not want to 
inherit the table background:

<TD background="">

Or, you can make the document render in standard mode instead of NavQuirks mode 
by adding a doctype at the top indicating you want the strict DTD, as in:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
"http://www.w3c.org/TR/html4/strict.dtd">

For NavQuirks, the behavior is correct so I am marking this closed again. If you 
really have issue with the quirk implementation, i.e. you think we should NOT 
implement the quirk, then please let me know and we can open another bug.
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 24

18 years ago
It works on Windows 95 build ID 2000051210 and Linux build ID 2000051210

Note that Windows 9x displays some weird behaviour in specific cases for
background images, but that seems to be unrelated to this bug. See bug 38406.

Marking verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.