Closed
Bug 163034
Opened 22 years ago
Closed 22 years ago
Wrong table rendering - 1.0 branch only
Categories
(Core :: Layout: Tables, defect, P2)
Core
Layout: Tables
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> </td> <td class="wob" align="center">Help</td><td class="wob"> | </td> <td class="wob" align="center">Test</td><td class="wob"> | </td> <td width="100%"> </td> </tr> </tbody></table> wrong rendering: <table cellspacing="0" cellpadding="0" width="100%" border="0"> <tbody><tr bgcolor="#000000"> <td> </td> <td align="center"><font color="#FFFFFF">Help</font></td><td> <font color="#FFFFFF">|</font> </td> <td align="center"><font color="#FFFFFF">Test</font></td><td> <font color="#FFFFFF">|</font> </td> <td width="100%"> </td> </tr> </tbody></table> </body> </html>
Comment 3•22 years ago
|
||
Confirmed on build 2002081506 (1.0.1rc1). Works ok on 2002080718-trunk. (both win32).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•22 years ago
|
||
Comment 5•22 years ago
|
||
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
Comment 8•22 years ago
|
||
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
Comment 10•22 years ago
|
||
(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 → ---
Comment 11•22 years ago
|
||
This seems to work ok (no bug) in a self-built Mozilla as of 20020701 - unless I have done my build incorrectly.
Comment 12•22 years ago
|
||
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!
Comment 13•22 years ago
|
||
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.
Comment 14•22 years ago
|
||
Stupid IE. Hopefully this link works: http://bonsai.mozilla.org/cvsquery.cgi?module=SeaMonkeyAll&branch=MOZILLA_1_0_BRANCH&branchtype=match&date=explicit&mindate=08%2F06%2F2002&maxdate=08%2F06%2F2002+11%3A00&cvsroot=%2Fcvsroot
Comment 15•22 years ago
|
||
We don't need FONT tags to demonstrate this bug, just a point at which we could line-break the string.
Comment 16•22 years ago
|
||
... if we remove the space between the nbsp and text, it works.
Comment 17•22 years ago
|
||
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
Comment 18•22 years ago
|
||
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).
Comment 19•22 years ago
|
||
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.
Comment 20•22 years ago
|
||
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.
Comment 21•22 years ago
|
||
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.
Updated•22 years ago
|
Severity: normal → major
Whiteboard: [adt2 RTM] [ETA Needed]
Target Milestone: --- → mozilla1.0.1
Comment 22•22 years ago
|
||
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.
Assignee | ||
Comment 23•22 years ago
|
||
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
Comment 24•22 years ago
|
||
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.
Assignee | ||
Comment 25•22 years ago
|
||
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.
Assignee | ||
Comment 26•22 years ago
|
||
Assignee | ||
Comment 27•22 years ago
|
||
boris, rbs, could you r/sr?
Comment 28•22 years ago
|
||
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.
Comment 29•22 years ago
|
||
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
Updated•22 years ago
|
Priority: -- → P2
Updated•22 years ago
|
Whiteboard: [adt2] [ETA 09/09] → [adt2] [ETA 09/11]
Comment 31•22 years ago
|
||
Is this patch ready to go into branch?
Assignee | ||
Comment 32•22 years ago
|
||
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.
Assignee | ||
Comment 33•22 years ago
|
||
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 ago → 22 years ago
Keywords: mozilla1.0.2 → fixed1.0.2
Resolution: --- → FIXED
Comment 34•22 years ago
|
||
Verified fixed on branch build : 2002-09-19-08-1.0 WIN2K. Adding keyword "verified1.0.2".
Keywords: verified1.0.2
Comment 35•22 years ago
|
||
Should the status of this move from RESOLVED to VERIFIED?
Comment 36•22 years ago
|
||
The status of this bug should be moved from RESOLVED to VERIFIED only when this bug is verfied on trunk builds.
Comment 37•22 years ago
|
||
amar, this bug only exists on the 1.0 branch. The equivalent trunk bug was bug 161328.
You need to log in
before you can comment on or make changes to this bug.
Description
•