Closed Bug 542620 Opened 14 years ago Closed 14 years ago

table sometimes ignore background-color of column

Categories

(Core :: Layout: Tables, defect)

Product:
Core
Component:
Layout: Tables
Version:
1.9.2 Branch
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- beta1+
status2.0 --- ?
blocking1.9.2 --- needed
status1.9.2 --- .2-fixed

People

(Reporter: runoutoftime, Assigned: roc)

References

Details

(Keywords: regression, testcase, verified1.9.2)

Attachments

(4 files, 1 obsolete file)

reporter's testcase
545 bytes, text/html
Details
fix
4.15 KB, patch
dbaron
: review+
beltzner
: approval1.9.2.2+
Details | Diff | Splinter Review
fix nsFrameList usage in previous patch
2.47 KB, patch
bzbarsky
: review+
beltzner
: approval1.9.2.2+
Details | Diff | Splinter Review
Current rendering of testcase from 2-25-2010
9.25 KB, image/png
Details 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; he; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; he; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)

<table> ignores background-color style of <col>.

However, background-color of <col> applies when I add a <tr> with another background-color.

Two tables (good and bad) are attached.

Reproducible: Always

Steps to Reproduce:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>title</title>
</head>
<body>
	<table>
		<col style="background-color: green"></col>
		<tr style="background-color: blue"><td>blue</td></tr>
		<tr><td>green</td></tr>
	</table>

	<table>
		<col style="background-color: green"></col>
		<tr><td>THIS SHOULD BE IN GREEN</td></tr>
	</table>
</body>
</html>

Actual Results:  
First table is ok (blue and green).

Second table is broken (not green)

Expected Results:  
Second table should be green

I believe it worked in FF 3.5
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20100127 Minefield/3.7a1pre

Confirmed, this worked indeed fine with Firefox 3.5 but is broken on trunk and with Firefox 3.6.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, regressionwindow-wanted, testcase
Version: unspecified → 3.6 Branch
Component: General → Layout: Tables
Product: Firefox → Core
QA Contact: general → layout.tables
Version: 3.6 Branch → 1.9.2 Branch
AnyTablePartHasBorderOrBackground needs to look at column frames, which are stored on a separate child list.
Assignee: nobody → roc
blocking1.9.2: --- → ?
Attached patch fix — — Splinter Review 
Indeed.
Attachment #423929 - Flags: review?(dbaron)
Whiteboard: [needs review]
Whiteboard: [needs review] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/5c6c40887584
Status: NEW → RESOLVED
Closed: 14 years ago
status2.0: --- → ?
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Comment on attachment 423929 [details] [diff] [review]
fix

fix for a layout regression
Attachment #423929 - Flags: approval1.9.2.1?
That patch would trigger assertions in a build with DEBUG_FRAME_LIST.  In particular, using some random non-last-sibling frame as the last frame of an nsFrameList is wrong.

The method here probably wants to take an nsFrameList::Slice, not an nsFrameList.
If I use Slice, then to construct a slice that contains just this table frame, I have to call GetParent()->GetChildList(nsnull) to get the framelist the table frame belongs to. (Fortunately we know that the table must be in the main child list of the table-outer.) It's awfully tempting to make AnyTablePartHasBorderOrBackground just take "start, end" nsIFrame* parameters. Would that be wrong? :-)
Attachment #424925 - Flags: review?(bzbarsky)
Comment on attachment 424925 [details] [diff] [review]
fix nsFrameList usage in previous patch

This looks fine.

Taking start,end frames is probably ok, since this method never _modifies_ the passed-in list.
Attachment #424925 - Flags: review?(bzbarsky) → review+
OK, revised to just use two frames. This should be more efficient and robust.
Attachment #423929 - Attachment is obsolete: true
Attachment #424925 - Attachment is obsolete: true
Attachment #425120 - Flags: review?(bzbarsky)
Attachment #423929 - Flags: approval1.9.2.2?
Attachment #425120 - Flags: review?(bzbarsky) → review+
Whiteboard: [needs landing]
Attachment #423929 - Attachment is obsolete: false
Attachment #423929 - Flags: approval1.9.2.2?
Attachment #425120 - Attachment description: fix → fix nsFrameList usage in previous patch
Attachment #424925 - Attachment description: fix → fix nsFrameList usage in previous patch
blocking1.9.2: ? → needed
status1.9.2: --- → wanted
This fixes a pretty bad regression in 3.6 from some last-minute changes, and we really shouldn't ship a major update to 3.6 without this patch; I think this would have been a clear release blocker if we'd seen it before the release.
Attachment #425120 - Flags: approval1.9.2.2? → approval1.9.2.2+
Comment on attachment 423929 [details] [diff] [review]
fix

a1922=beltzner, yay tests!
Attachment #423929 - Flags: approval1.9.2.2? → approval1.9.2.2+
Whiteboard: [needs 192 landing]
I tested this with the attached testcase on Firefox 3.6.0 and my 3.6.2 build. I see no difference in rendering (which appears correct on Firefox 3.6.0). Both render as this attachment does.

So, the testcase issue isn't something that we fixed.
What changeset is your 3.6.2 build built on?  (See the SourceStamp line in application.ini .)
I can confirm that the testcase doesn't work with a freshly made build, at
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/18ea3b9ac710

when I look at the push log, I see that that those checkins are tagged with 'GECKO1921_20100126_RELBRANCH'
http://hg.mozilla.org/releases/mozilla-1.9.2/summary
verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.3pre) Gecko/20100319 Namoroka/3.6.3pre
Keywords: verified1.9.2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: