Closed Bug 163034 Opened 22 years ago Closed 22 years ago

Wrong table rendering - 1.0 branch only

Categories

(Core :: Layout: Tables, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.0.1

People

(Reporter: tammer, Assigned: shanjian)

References

Details

(Keywords: regression, topembed+, Whiteboard: [adt2] [ETA 09/11])

Attachments

(6 files, 1 obsolete file)

Hello,
the following example works with Mozilla 1.0, IE 5.5, IE 6.0, Opera x.x,
Netscape 4.x. Unfortunately Mozilla 1.0.1rc1 is broken. The FONT tag destroys
the table.

Bye
  Rainer Tammer

P.S.: Sample

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html>
<head>
  <title>Test Table</title>
  <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
  <style type="text/css">
  <!--
  .wob { font-weight: normal; background-color: #000000; color: #ffffff; }

  caption,th,td,tr { font-family: Arial, sans-serif; font-size: 10pt; }

  body { font-size: 10pt;  margin-top: 0%; margin-left: 0%; color: black;
margin-right: 0%; font-family: serif; background-color: white; }

  -->
  </style>
</head>
<body text="#000000" vlink="#551a8b" alink="#ff0000" link="#0000ee"
bgcolor="#ffffff" leftmargin="0" topmargin="0" rightmargin="0" marginheight="0"
marginwidth="0">

right rendering:
<table cellspacing="0" cellpadding="0" width="100%" border="0">
  <tbody><tr bgcolor="#000000">
    <td>&nbsp;</td>
    <td class="wob" align="center">Help</td><td class="wob">&nbsp;|&nbsp;</td>
    <td class="wob" align="center">Test</td><td class="wob">&nbsp;|&nbsp;</td>
    <td width="100%">&nbsp;</td>
  </tr>
</tbody></table>

wrong rendering:
<table cellspacing="0" cellpadding="0" width="100%" border="0">
  <tbody><tr bgcolor="#000000">
    <td>&nbsp;</td>
    <td align="center"><font color="#FFFFFF">Help</font></td><td>&nbsp;<font
color="#FFFFFF">|</font>&nbsp;</td>
    <td align="center"><font color="#FFFFFF">Test</font></td><td>&nbsp;<font
color="#FFFFFF">|</font>&nbsp;</td>
    <td width="100%">&nbsp;</td>
  </tr>
</tbody></table>


</body>
</html>
Confirmed on build 2002081506 (1.0.1rc1).  Works ok on 2002080718-trunk. (both 
win32).
Status: UNCONFIRMED → NEW
Ever confirmed: true
WFM (2002081604, WinNT)
OS: All.

is OK on current trunk CVS, Linux, but looks bad (or like reporter states, and
as in attachment 95568 [details]) when testing with mozilla 1.0.1rc1 2002081507  Linux.
OS: Windows NT → All
Hardware: PC → All
WFM 2002081604/winXP
We *know* this works on the trunk.  Please stop spamming the bug with me-too
WFM-on-trunk comments.
Summary: Wrong table rendering → Wrong table rendering - 1.0 branch only
Mark it as WFM
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → WORKSFORME
(Sigh) Reopening.  Does no-one read comments anymore?

To reiterate, this *works on the trunk*.

Here is a summary of which builds exhibit this bug (all Win32 unless otherwise 
stated):

1.0 branch:
Mozilla 1.0 release (2002053012) works.
1.0.1rc1 (2002081506 on Win32, 2002081507 on Linux) is broken.
1.0 branch latest (2002081806) is broken.

1.1 branch:
1.1 branch latest (build id has been hidden, but 20020818xx) works.

trunk:
trunk latest (2002081808) works.

So this bug is present only on the 1.0 branch, and was introduced somewhere 
between 2002053012 and 2002081506.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
This seems to work ok (no bug) in a self-built Mozilla as of 20020701 - unless I
have done my build incorrectly.
This works ok (no bug) in a self-build of Mozilla pulled as at 20020731 01:00
PDT and fails (as reported) in a build as at 20020808 15:00 PDT (14:00 PST).

There are a couple of likely candidates, but I'll go through them tomorrow - I
need to go to sleep now!
This was caused by the checkin for bug 160547:

http://bonsai.mozilla.org/cvsquery.cgi?
module=SeaMonkeyAll&branch=MOZILLA_1_0_BRANCH&branchtype=match&date=explicit&min
date=08%2F06%2F2002&maxdate=08%2F06%2F2002+11%3A00&cvsroot=%2Fcvsroot

I took a build as of 2002/08/08 15:00 PDT (which exhibits the problem) and 
rolled back nsTextFrame.cpp to version 1.363.2.5, undoing the above checkin.  
This fixes the problem (and naturally regresses bug 160547).

The command I used was:
cvs update -j1.363.2.6 -j1.363.2.5 mozilla/layout/html/base/src/nsTextFrame.cpp

I'm just double-checking this by updating to the tip of the MOZILLA_1_0_BRANCH.
Attached file Minimal testcase (obsolete) —
We don't need FONT tags to demonstrate this bug, just a point at which we could
line-break the string.
... if we remove the space between the nbsp and text, it works.
Attached file Minimal testcase
Oops.  I should have tested that more thoroughly.  We always do linebreaking at
whitespace.  This testcase shows that we now do it at tags too (<SPAN>, here).
Attachment #96007 - Attachment is obsolete: true
So this is what I think is happening:

1. The last table cell expands to fill as much horizontal space as is 
available, forcing the other cells to be the minimum width possible.

2. We now decide to linebreak the cells between the nbsp and the text, because 
we now consider the <SPAN> or <FONT> tag to be a valid linebreak position.  If 
you place some text before the nbsp, you can see this clearly.  This pushes the 
following text onto the next line.

I'm not certain whether this is incorrect behaviour according to the spec, but 
it is definitely a change from previous behaviour (and from IE's behaviour).
Now I'm confused.

The latest 1.0 nightly build (2002081906) still exhibits this problem, but my
self-built binary using the 1.0 tip doesn't.  I suppose it's possible that
something got checked recently in that fixes the problem, though I can't see it.

I'll wait until later to try a later nightly build.
Ok, I worked out what I did wrong with my build, and I've confirmed that the 
bug is still present in the latest MOZILLA_1_0_BRANCH tip, and that reverting 
the checkin for bug 160547 definitely fixes it.

Now that I understand this a little better, I'm unsure whether this is a 
genuine bug or not.  On one hand, I don't know if the HTML spec details where 
line-breaking can and cannot occur.  On the other hand, we've changed the way 
we do line-breaking so that we no longer render the same as MSIE and older 
versions of Mozilla (and Netscape Navigator and Opera, according to the 
original reporter).

I checked a bunch of top100 sites, and we don't seem to have broken anything 
obvious, though.
Addendum (sorry for the spam).  Since this odd behaviour exists only on the 1.0 
branch, I'm inclined to believe that it is a geniune bug.  I'm reopening bug 
160547 and setting this one as a dependency.
Depends on: 160547
Severity: normal → major
Whiteboard: [adt2 RTM] [ETA Needed]
Target Milestone: --- → mozilla1.0.1
add  momoi@netscape.com to the cc list
please do top site testing on m1.0 branch and see how bad is this regression
how many top sites got impact by this bug
shanjian said this is a regression from 6.2 also. so we fix one regression and
cuase this one. 
Just as rbs pointed out in bug 160547, the regression is because we failed to do
the "RevertSpacesToNBSP" before line break. To check if the 2 connecting piece
is breakable in between is absolutely correct. Since this is a linebreaker call,
"RevertSpacesToNBSP" should also be called in this case. As a result,
"RevertSpacesToNBSP" should be called regardless of "aCanBreakBefore"

A new patch is proposed in bug 161328 for trunk to solve all the problems
mentioned in this bug, bug 160547, and bug 161328.

I can develop a patch for branch only if necessary. 
Status: REOPENED → ASSIGNED
Shanjian, this bug does not exist in the trunk.  Bug 160547 was only checked in 
to the m1.0 branch, and caused this bug.

The patch currently attached to bug 161328 will therefore not resolve either 
bug 160547 or this one.
I knew original patch for bug 160547 was only checked into branch. So let me
state this way. 
Old patch in 161328 include patch of 160547, if checked in trunk, it would have
cause regression mentioned in this bug.
New patch in 161328 also include patch of 160547,and addesses the issue
mentioned in this bug. 

For trunk, once patch for 161328 is checked in, all 3 bugs can be resolved.
For branch, we need to have a new patch base on patch in 161328 if we decide to
fix it.
boris, rbs, could you r/sr?
Any urgency here? Let's wait for the trunk patch (bug 161328) and you could
possibly migrate the full patch if all goes well -- the same patch will allow
catching troubles more quickly. This regression would have been caught pretty
quickly if it was on the trunk.
Shanjian, thanks for the explanation.  My point was just that checking in the
trunk fix for bug 161328 will not resolve this bug.  From your comments, it was
unclear whether you were intending to produce a branch fix, or whether you were
suggesting we resolve this bug as WONTFIX.

The fact that we have a branch patch makes that question moot.

Per rbs' comment 28, setting a dependency on the trunk patch, bug 161328.
Depends on: 161328
->shanjian
Assignee: karnaze → shanjian
Status: ASSIGNED → NEW
Blocks: 154896
Whiteboard: [adt2 RTM] [ETA Needed] → [adt2] [ETA 09/06]
Priority: -- → P2
Keywords: edt1.0.2
Whiteboard: [adt2] [ETA 09/06] → [adt2] [ETA 09/09]
Whiteboard: [adt2] [ETA 09/09] → [adt2] [ETA 09/11]
Is this patch ready to go into branch?
the patch here will not go to branch, but patch for 161328 will. Once it does,
the problem mentioned in this report will be resolved in branch.
Marked this one as fixed. patch for 161328 has been checked into to branch,
which will address the problme mentioned here. 
Status: NEW → RESOLVED
Closed: 22 years ago22 years ago
Keywords: mozilla1.0.2fixed1.0.2
Resolution: --- → FIXED
 Verified fixed on branch build : 2002-09-19-08-1.0 WIN2K. Adding keyword
"verified1.0.2".
Keywords: verified1.0.2
Should the status of this move from RESOLVED to VERIFIED?
The status of this bug should be moved from RESOLVED to VERIFIED only when this 
bug is verfied on trunk builds.
amar, this bug only exists on the 1.0 branch.  The equivalent trunk bug was bug
161328.
verified on branch 2003-06-03-05trunk winXP
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: