Last Comment Bug 5821 - {compat} Nav4 vs CSS2 line box model
: {compat} Nav4 vs CSS2 line box model
Status: VERIFIED FIXED
[NOTESTCASENEEDED]
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Windows 98
: P2 critical (vote)
: M14
Assigned To: buster
: Christine Hoffman
Mentors:
http://www.barnesandnoble.com
: 2971 3016 4376 4677 4769 5031 5775 5834 5851 5900 7118 7290 7581 7675 8180 8466 8534 8757 9244 9402 9481 11243 11371 11529 12605 12744 12786 13045 14440 15004 15543 28133 28196 (view as bug list)
Depends on: 6865
Blocks:
  Show dependency treegraph
 
Reported: 1999-04-30 19:26 PDT by cpeterso
Modified: 2003-12-08 12:07 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
linked images create too big table cells - testcase (595 bytes, text/html)
1999-06-03 15:11 PDT, Robert Kaiser (not working on stability any more)
no flags Details
An even simpler example w/o table. (359 bytes, text/html)
1999-09-02 16:31 PDT, kipp
no flags Details
patch described in comments (1.85 KB, patch)
1999-09-02 18:40 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
New testcase, simplified version of www.citec.fi (419 bytes, text/html)
2000-04-18 15:35 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details

Description rickg 1999-05-03 11:28:59 PDT
Chris -- another table problem. Here's the min. case:

<html>
<body>
  <table width="100%" cellspacing="0" cellpadding="0" border="0">
    <tr>
      <td>
        <table cellspacing="0" cellpadding="0" border="0">
          <tr>
            <td bgcolor="#003366"><img src="/gresources/cleardot.gif" alt="."
width="1" height="3" border="0"></td>
          </tr>
          <tr>
            <td bgcolor="#003366">Books &nbsp;Home</td>
          </tr>
        </table>
      </td>
    </tr>
  </table>
</body>
</html>
Comment 1 karnaze (gone) 1999-05-03 15:12:59 PDT
Here is a simpler example illustrating why the table cell/row is too tall.
During nsTableCellFrame::Reflow() on line 532, the area frame (1st child of
nsTableCellFrame) is returning a desired height bigger than the 3 pixels that it
should be.

<html>
<body>
 <table cellspacing="0" cellpadding="0" border="1">
  <tr>
   <td bgcolor=white><img src="raptor.jpg" width="1" height="3"></td>
  </tr>
</table>
</body>
</html>
Comment 2 kipp 1999-05-03 18:47:59 PDT
Yet another example of a conflict between nav compatability and css2's
line-height and line-layout specification.
Comment 3 Hixie (not reading bugmail) 1999-05-19 14:36:59 PDT
This could be fixed by applying vertical-align:bottom to images that are in a
content model containing only images.

So, for example, the following:

   <div>
     <img ...>
     <img ...>
     <img ...>
   </div>

would imply vertical-align:bottom for those images, while the following:

   <div>
     <img ...>
     is good.
   </div>

...would use vertical-align:baseline (the initial value), aligning the images
with the text baseline.

A simpler fix would be achieved by using the following rule in the
compatibility mode ua.css:

   IMG { vertical-align: bottom; }

Of course, that would also cause images that are mixed with text to be slightly
too low, hence the more complex solution given above.

A study of 4.x behaviour may be needed to ascertain the best solution, though.
Comment 4 Hixie (not reading bugmail) 1999-05-20 06:06:59 PDT
*** Bug 4769 has been marked as a duplicate of this bug. ***
Comment 5 Hixie (not reading bugmail) 1999-05-20 06:08:59 PDT
*** Bug 4376 has been marked as a duplicate of this bug. ***
Comment 6 Hixie (not reading bugmail) 1999-05-20 06:10:59 PDT
[ccing dbaron as he was on 4769's cc list]

Created an attachment (id=25)
Series of test cases that were attached to 4769, showing simplified form of
problem and its cause.
Comment 7 Hixie (not reading bugmail) 1999-05-24 08:30:59 PDT
*** Bug 5834 has been marked as a duplicate of this bug. ***
Comment 8 Hixie (not reading bugmail) 1999-05-27 15:23:59 PDT
*** Bug 7118 has been marked as a duplicate of this bug. ***
Comment 9 Hixie (not reading bugmail) 1999-05-27 15:25:59 PDT
Bug 7118 pointed to the small "GO" logo at the top of
   http://abcnews.go.com/
Comment 10 Hixie (not reading bugmail) 1999-05-29 03:16:59 PDT
*** Bug 7290 has been marked as a duplicate of this bug. ***
Comment 11 Hixie (not reading bugmail) 1999-05-29 03:17:59 PDT
*** Bug 5900 has been marked as a duplicate of this bug. ***
Comment 12 Hixie (not reading bugmail) 1999-05-29 03:17:59 PDT
Bug 5900 points to the following, which should be checked when fixing and
verifying this bug:

   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=120
   more duplicates: bug 6140, bug 6631, bug 5528, bug 6237, bug 7336
Comment 13 Robert Kaiser (not working on stability any more) 1999-06-03 15:11:59 PDT
Created attachment 335 [details]
linked images create too big table cells - testcase
Comment 14 Robert Kaiser (not working on stability any more) 1999-06-03 15:14:59 PDT
added attachment (testcase originally from #7290),
added me to cc-list.

I hope it can be fixed before M15...
Comment 15 Hixie (not reading bugmail) 1999-06-04 06:07:59 PDT
*** Bug 7581 has been marked as a duplicate of this bug. ***
Comment 16 Hixie (not reading bugmail) 1999-06-04 06:17:59 PDT
KaiRo@StarTrekMail.com: The M15 marker is in place because kipp@netscape.com
is on sabbatical. This compatability issue will likely be addressed when he
returns, in around a month's time.

Also, please note that *this is not a bug*. Mozilla is acting correctly in all
these cases. It is the web pages that are incorrect. Mozilla is rendering the
pages exactly as per the CSS2 specification.

[I just realised I set the priority to P1, which kipp reservers for crashers.
 Oops. Reducing priority to P2.]
Comment 17 John Morrison 1999-06-06 11:15:59 PDT
*** Bug 7675 has been marked as a duplicate of this bug. ***
Comment 18 Hixie (not reading bugmail) 1999-06-10 11:56:59 PDT
*** Bug 7352 has been marked as a duplicate of this bug. ***
Comment 19 Hixie (not reading bugmail) 1999-06-18 14:45:59 PDT
*** Bug 8466 has been marked as a duplicate of this bug. ***
Comment 20 Daniel Roberts 1999-06-19 20:26:59 PDT
*** Bug 8534 has been marked as a duplicate of this bug. ***
Comment 21 Daniel Roberts 1999-06-19 20:28:59 PDT
You might also want to look at linux.com as per bug 8534 when this bug gets
fixed as an additional test case.
Comment 22 Hixie (not reading bugmail) 1999-06-23 13:40:59 PDT
*** Bug 8180 has been marked as a duplicate of this bug. ***
Comment 23 Hixie (not reading bugmail) 1999-06-23 17:56:59 PDT
*** Bug 5851 has been marked as a duplicate of this bug. ***
Comment 24 Hixie (not reading bugmail) 1999-06-23 18:56:59 PDT
*** Bug 5031 has been marked as a duplicate of this bug. ***
Comment 25 Hixie (not reading bugmail) 1999-06-23 18:57:59 PDT
*** Bug 5031 has been marked as a duplicate of this bug. ***
Comment 26 John Morrison 1999-06-24 07:30:59 PDT
*** Bug 8757 has been marked as a duplicate of this bug. ***
Comment 27 Andrew Niese 1999-06-28 16:55:59 PDT
Just wondering what the status is on this puppy getting fixed.
kipp's last comments were on 05/03/99 .. almost 2 months ago.
Comment 28 Hixie (not reading bugmail) 1999-06-28 17:16:59 PDT
Fixed? Our current behaviour is correct!

Kipp is currently on sabbatical. I believe he should be back soon. This
compatability issue will likely be addressed when he returns.
Comment 29 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-06-29 15:53:59 PDT
Marking NOTESTCASENEEDED.  The problem is well understood.  The issue is what to
do about it, and what fix would be compatible enough.
Comment 30 John Morrison 1999-07-03 18:16:59 PDT
*** Bug 9244 has been marked as a duplicate of this bug. ***
Comment 31 John Morrison 1999-07-07 20:01:59 PDT
*** Bug 9402 has been marked as a duplicate of this bug. ***
Comment 32 John Morrison 1999-07-07 20:08:59 PDT
*** Bug 9402 has been marked as a duplicate of this bug. ***
Comment 33 Sam Allen 1999-07-08 20:26:59 PDT
*** Bug 9481 has been marked as a duplicate of this bug. ***
Comment 34 Nicholas Cull 1999-07-09 21:20:59 PDT
*** Bug 4677 has been marked as a duplicate of this bug. ***
Comment 35 Nicholas Cull 1999-07-09 21:34:59 PDT
*** Bug 3016 has been marked as a duplicate of this bug. ***
Comment 36 Nicholas Cull 1999-07-09 21:35:59 PDT
*** Bug 3016 has been marked as a duplicate of this bug. ***
Comment 37 Péter Bajusz 1999-07-11 11:52:59 PDT
I was suprised to see that noone reacted on Ian's 05/19/99 comment, so I do
it now. I'm talking about the "vertical-align:bottom" solution, which has
its problems. First of all it does not fix the problem...
It is only producing a looks-like-expected layout, if the line-height is
smaller than the height of the image. Now this is one of the first things
a web designer has to find out not to rely upon.

Something to clarify about this bug:
In current versions (like 1999-07-10-08) there's no such gap when the
content is only an image. There's only a gap when the image is wrapped in
an "empty" inline-element, such as an anchor.
The attachment which was added by Ian is showing that.

The problem is that while we expect the cell to end up as high as the image,
the inline element streches it. If it's big enough it can stretch the cell
regardless of the aligment of the image.

This is indeed a serious issue, because it appears that mozilla not only
"breaks" existing pages, but there's no standard compliant way to
modify such a web page to produce the desired result.

I've studied CSS2 for a few hours (which is not much I can agree) to find out
about this. Here's what seems to apply:

In chapter 10.6.1 Inline, non-replaced elements:
... The 'height' property doesn't apply, but the height of the box is given
by the 'line-height' property.

In chapter 10.8.1 about the calculation of "line-height":
[it should by default to be set] to a "reasonable" value based on the
font size of the element.

In 15.2.4 it says about the font-size:
The _actual_value_ of this property may differ from the _computed_value_ due a
numerical value on 'font-size-adjust' and the unavailability of certain font
sizes.

So if we want to attack this we should argue about what's "resonable" or
that we needs a deviation of the font-size's actual value from the computed
value.

I've programmed some simple text layout program myself and I came up
calculating line-height as the maximum of the line's glyps' ascent plus
the maximum of the line's glyps' descent. Since ascent and descent is
(by definition?) the same in all glyps in a font, so line-height is the maximum
of the line's fonts' ascent plus the maximum of the line's fonts' descent.
What is one may think at first is that an inline element will use only
one font, but that's not actually true. It is possible that different
characters in the element are requiring different fonts because of different
encoding. (NS4 didn't support this but IS4 and mozilla is.) Because the font
inavailibility the font-size may different with each font used, hence the
maximum calculation is needed.
Question is: what is the maximum of the "height of the glyps" (simplified)
if there are no glyps as there are no characters in the element.
The standard say to pick a "resonable" value, which may differ from the
computed one. I'd say pick zero in that case. It's a _resonable_ value,
because it ensures compatibility.

Let's make it clear, I didn't talk about EMPTY element's. I talked about
elements with no characters in them. The element may contain inline elements.
Of course if an element contains no characters, but it contains an inline
element that does the inside element could strech the hight of the outside
but I will investigate that more (both standard-wise and compatibility-wise).

Oh yes, just my $0.02
Comment 38 Hixie (not reading bugmail) 1999-07-17 07:44:59 PDT
hyp-x@inf.bme.hu: I disagree. IMHO, the vertical-align:bottom solution _is_ a
satisfactory solution.

You say:
> In current versions (like 1999-07-10-08) there's no such gap when the
> content is only an image. There's only a gap when the image is wrapped in
> an "empty" inline-element, such as an anchor.

This is bug 6865, which I am adding as a dependency.


> It is only producing a looks-like-expected layout, if the line-height is
> smaller than the height of the image.

Which is absolutely correct. Do any of the dozens of dups and test cases for
this bug have a line-height bigger than the images?


Note also that your method of calculating line-height on a per line basis is
all very well, but it is also completely non-spec compliant. The CSS specs are
actually quite thorough in their definition of how to calculate line heights.
Comment 39 Péter Bajusz 1999-08-03 11:04:59 PDT
*** Bug 2971 has been marked as a duplicate of this bug. ***
Comment 40 Nicholas Cull 1999-08-05 02:20:59 PDT
*** Bug 11243 has been marked as a duplicate of this bug. ***
Comment 41 Péter Bajusz 1999-08-08 09:52:59 PDT
*** Bug 11371 has been marked as a duplicate of this bug. ***
Comment 42 Péter Bajusz 1999-08-08 09:56:59 PDT
Kipp just accepted 11371 for M10, which is a duplicate,
so I'm marking this one M10.
Comment 43 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-08-14 17:32:59 PDT
*** Bug 11529 has been marked as a duplicate of this bug. ***
Comment 44 Eli Goldberg 1999-08-27 09:17:59 PDT
*** Bug 12605 has been marked as a duplicate of this bug. ***
Comment 45 Mats Palmgren (:mats) 1999-08-30 10:17:59 PDT
*** Bug 12786 has been marked as a duplicate of this bug. ***
Comment 46 kipp 1999-09-01 08:10:59 PDT
*** Bug 5775 has been marked as a duplicate of this bug. ***
Comment 47 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-01 19:31:59 PDT
*** Bug 12744 has been marked as a duplicate of this bug. ***
Comment 48 Mats Palmgren (:mats) 1999-09-02 13:15:59 PDT
*** Bug 7529 has been marked as a duplicate of this bug. ***
Comment 49 Mats Palmgren (:mats) 1999-09-02 15:47:59 PDT
*** Bug 13045 has been marked as a duplicate of this bug. ***
Comment 50 kipp 1999-09-02 16:31:59 PDT
Created attachment 1521 [details]
An even simpler example w/o table.
Comment 51 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-02 16:40:59 PDT
One way to fix this would be to (in compat mode only) make inline boxes that do
not have text nodes as *children* have zero height and sit on the text
baseline.  Would that work?
Comment 52 Hixie (not reading bugmail) 1999-09-02 17:13:59 PDT
David: If bug 6865 is fixed by implementing your anonmyous-inline-around-all-
inlines-in-a-block solution, then yes, doing this would work. (Note that only non-
replaced inline elements should have their heights shrunk in this way -- the
images must still have their height to give the line box a height!)

Kipp: I *strongly* recommend fixing bug 6865 first. Doing so should be relatively
easy: you just have to insert an anonymous inline element around all inline
content in a block. For example, the following:

   <div>
      <span> abc </span>
      <span> def </span>
   </div>

...would internally become

   <div>
      <anonymous-inline>
         <span> abc </span>
         <span> def </span>
      </anonymous-inline>
   </div>

See http://lists.w3.org/Archives/Public/www-style/1999Jan/0027.html for an
explanation of this problem and the proposed solution.

Once this is implemented, then this bug can be fixed completely by implementing
the mechanism that David has just proposed:

   In compat mode only, make non-replaced inline boxes that do not have text
   nodes as children have zero height and sit on the text baseline.

...or, in pseudo-css:

   @compat-mode {
      :inline:contains(:text-node) { line-height: 0; }
   }

I believe this is a complete solution, and does not require any further changes.
It requires _no_ changes to strict mode at all.
Comment 53 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-02 18:35:59 PDT
I started poking around in the code, and I think I may have the beginning of a
fix for this.  It has the following problems:

 * I don't know how to detect compatMode, so I made a variable called compatMode
and set it to PR_TRUE.  It should be initialized correctly (true if quirks,
false if standard).
 * I don't handle ignorable whitespace properly.  This means if there is any
space like <a ... > <img></a> rather than <a ...><img></a> it doesn't work.

A few more notes:
 * It's against a source pull from Saturday
 * I tested it a bit side-by-side with an opt build from Monday.
 * I ran through 5 of my CSS tests (linebox[1-4] and inlinetest) and a number of
the bugs (5-7??) that were resolved as dups of this one.
 * It seemed to fix all but one of the bugs I tested here (the whitespace
issue), except that http://www.linux.com/ and http://www.barnesandnoble.com/
didn't seem to load quite properly in either.  I was getting errors in the debug
build but not the opt build, so it's hard to tell if I caused that.
 * Among the 5 of my tests I tested, the only regression I saw was in linebox2
(middle test, I think).  If this is compat mode only, then I think that's OK.
It's what I would expect to happen...
 * It does need a bit more testing, I think, and of course you should look at it
carefully since I don't completely know what I'm doing (especially C vs. C++
issues like where I should declare the variables - I've never really done much
C++ as opposed to C).
 * So that the diffs are easier to read as diffs, I didn't make an indenting
change in about 50 lines of code that need an additional (2-space) tab of
indenting.
Comment 54 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-02 18:40:59 PDT
Created attachment 1523 [details] [diff] [review]
patch described in comments
Comment 55 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-02 18:48:59 PDT
Now that I think about it, you might want to make this depended on the inline
element having no padding, border, or margin, and possibly also having
vertical-align of baseline.  It could cause some strange things otherwise.
Comment 56 kipp 1999-09-02 19:41:59 PDT
Well a quick test of david's suggested solution (my test was simpler - I always
set the height/ascent/descent to zero :-) lead to the right compatabile
behavior. Ignoring the other issues regarding the spec, this is kinda nice. So
I'm coding up a proper version that takes into account text-frames and
borders/padding/etc....More later.
Comment 57 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-02 20:12:59 PDT
You probably noticed this already, but, just for the record, I noticed that I
used two boolean variables where I should have used one... compatMode and
noTextChildren should be replaced by something like collapseHeight.  I'm not
that bad once I get a chance to look at (really, think about, since I was
thinking about the p/b/m and vertical-align stuff) what I've done :-).
Comment 58 kipp 1999-09-02 20:26:59 PDT
Okie dokie. I've tested every regression that this page links to with my coding
of david's suggestion and it *works*. The other case where there was another
problem I reopened.

Yippee... The implementation pays attention to the "dtd mode" and if we are in
compatability mode (most likely currently) then it behaves like nav.

Fix landing shortly.
Comment 59 kipp 1999-09-02 20:48:59 PDT
Changes checked in. Try out tommorows build and let me know...
Comment 60 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-04 12:44:59 PDT
This bug appears to be fixed.  However, verifying this should be a good bit of
work, because we need to check all the test cases in case some of them were not
truly duplicates of this bug or in case some of them weren't fixed by the
current fix.

It would be helpful if anyone who wants to help could go down (starting at the
top, or we'll get way too confused!) and write comments here that you've checked
things out or that you've reopened a bug because there are other problems.

(I'm going to investigate the problems I'm seeing on
http://www.barnesandnoble.com/ and http://www.linux.com/ in a few minutes.  I'll
probably look at some other cases too...)
Comment 61 Péter Bajusz 1999-09-05 03:54:59 PDT
I've started verification from the top:

4769
  The page has a <BODY><P> margin problem, covered in another bug.
4769 -> 5080
  It only has an unrelated table problem (table bgcolor)
  I'll look into this.
4376
  It has a residual style problem (<TABLE> inside <P>) remained.
5834
  The author changed the page to work without the fix.
7118
  It still has some table problems.
  I'll look into this.
7290
  It's rather invalid than a duplicate, because the table cell has an extra
  whitespace in which case both Nav4 and IE4 renders things "correctly".
  (Even so IE4 doesn't render the whitespace itself.)
  I'm leaving the bug alone. (RIP)
5900 is looking good
  It has a table alignment problem down at the applet.
  I have a testcase, I'll file a bug for that (or look-up an existing one.)
5900 -> 6140
  No problems with this site.
5900 -> 6631
  Page no longer exist.
5900 -> 5528
  No problems with this site.
5900 -> 6237
  No problems with this site.
5900 -> 7336
  No problems with this site.
Comment 62 Péter Bajusz 1999-09-05 06:00:59 PDT
7581
  No problems with this site.
7675
  No problems with this site.
7352
  Kipp reopened this bug for another problem.
  I made a testcase.
  This is the same problem I found on 5900, so that bug can be left alone.
8466
  Page no longer exists.
8534
  Dbaron is doing this (www.linux.com)
8180
  No problems with this site.
5851
  No problems with this site.
5031
  No problems with this site.
8757
  Site has a well known residual style problem.
9244
  No problems with this site.
9402
  Dbaron is doing this (www.linux.com)
9481
  No site URL, just testcase. It's OK.
4677
  No problems with this site.
3016
  Site URL is internal, testcases OK
2971
  The site has other bugs (well quirks) already covered elsewhere.
11243
  Page no longer exists.
11371
  No site URL, just testcase. It's OK.
11529
  No site URL, just testcase. It's OK.
12605
  No problems with this site.
12786
  No problems with this site.
5775
  No problems with this site.
5775 -> 8273
  I see an "entity in invalid table area" parsing problem.
  I was just about to file a bug for this, anyway. :)
5775 -> 9692
  No problems with this site.
5775 -> 10009
  There's a table problem on the site.
  I've reopened the bug.
5775 -> 10100
  There's a table problem on the site.
  I'll look into this.
5775 -> 10345
  I see covered residual style problems and the table alignment problem (7352)
5775 -> 10835
  No problems with this site.
12744
  I have a totally unrelated behaviour problem with the site.
  I'll look into it.
7529
  No problems with this site.
13045
  No problems with this site.

Huhh. That should do it.
One more doesn't matter, so I checked www.linux.com too.
I see no problem there.

I'm marking this one verified.
Comment 63 Hixie (not reading bugmail) 1999-09-05 13:06:59 PDT
hyp-x@inf.bme.hu: Impressive. Nice work. :-)
Comment 64 Mats Palmgren (:mats) 1999-09-06 09:36:59 PDT
*** Bug 13198 has been marked as a duplicate of this bug. ***
Comment 65 Hixie (not reading bugmail) 1999-09-08 13:37:59 PDT
The fix for this bug has bug 13097.
Comment 66 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-22 11:18:59 PDT
*** Bug 14440 has been marked as a duplicate of this bug. ***
Comment 67 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-09-27 20:31:59 PDT
*** Bug 15004 has been marked as a duplicate of this bug. ***
Comment 68 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 1999-10-05 12:17:59 PDT
*** Bug 15543 has been marked as a duplicate of this bug. ***
Comment 69 jst 2000-02-17 11:27:13 PST
This one is back, or not exactly this one but very close and since just about
every bug similar to this one has been marked a dup of this I'm reopening this
bug. The below html shows the new problem, the problem is that there's an about
3 pixel high gap between the two images, if I remove the 'a' that wraps the
first image then the extra space goes away. I first noticed this on
http://www.citec.fi.

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html>
<body>
<table border="0" cellpadding="0" cellspacing="0">
<tr> 
 <td valign="bottom">
  <a href="http://www.mozilla.org">
   <img src="http://www.mozilla.org/images/mozilla-banner.gif"
    border="0"></a></td>
</tr>
<tr>
 <td valign="top">
  <img src="http://www.mozilla.org/images/mozilla-banner.gif"
   border="0">
 </td>
</tr>
</table>
</body>
</html>

dbaron, I verified that this problem is caused by the modifications you did
on 02/14/2000 to nsLineLayout, could you have a look at that, backing out those
changes makes the testcase look like it does in NS4.7.
Comment 70 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-02-17 11:42:12 PST
I'm sure I tested a bunch of the testcases for this bug, but now I realize it
was probably not with the final version of the code that I checked in (since I
found and fixed a few bugs in it, and went through most, but not all, of the
testing over again).

Steve - do you think I should back the changes out?  I may have a chance to look
at this this evening, but I won't have all that much time.  Do you know what the
schedule for M14 is?
Comment 71 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-02-17 12:36:12 PST
Right now this problem is happening within table cells, but not within divs.  I
don't know why that is.  I don't think there's something strange about how
things work inside of table cells, but perhaps there is.  I suspect it's the
same thing as what's causing bug 15933.
Comment 72 Robert Kaiser (not working on stability any more) 2000-02-17 12:57:52 PST
Hey, my testcase I made in the good old days for #7290 (a dupe) - the testcase
ist the one included here called "linked images..." - behaves strange:

When it replaces the image with the string "testpic1" because it can't find the
image, the page reflows and the table height gets correct. If I copy it to a
local directory and put an image called "testpic1.gif" there, it shows the
picture and the table height is false.

Is there any conflict between quirks and strict mode? I assumed from this bug
discussion that for some strange reason the bigger cells are right in strict
mode but they are surely false in quirks mode...
Comment 73 Nisheeth Ranjan 2000-02-17 15:21:37 PST
*** Bug 28196 has been marked as a duplicate of this bug. ***
Comment 74 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-02-17 18:52:36 PST
OK... I've refixed this.  I'd like to test a bit more before I check in. 
However, I'm posting the patch for review now because I'm pretty sure it's
right, and I'd like to get a code review (and then approval) as soon as possible
so this can get fixed again.

The problem was that I was making the wrong comparison when seeing if the bottom
edge of a span needed to be shrunk.  The code was wrong when the span's children
extended out of it on top, but did not reach its edge on the bottom.  The point
where the top of the span's box should be is 0, so maxY is the lowest bottom
among the span's children, and minY is the highest top.  The bottom of the span
needs to be shrunk if its height is bigger than maxY, not when it's bigger than
(maxY-minY), since 0 is the top of the span.  I'll save you the explantion of
why I think I wrote what I did...
Comment 75 Mats Palmgren (:mats) 2000-02-18 08:56:11 PST
*** Bug 28133 has been marked as a duplicate of this bug. ***
Comment 76 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-02-18 19:46:19 PST
(Re)fix checked in 2000-02-18 19:42-0800.

Changing TM M10 -> M14.
Comment 77 jst 2000-02-21 16:14:08 PST
Verified this on both WinNT and Linux, things look very good now, way to go
David!
Comment 78 Johnny Stenback (:jst, jst@mozilla.com) 2000-04-18 15:35:28 PDT
Created attachment 7702 [details]
New testcase, simplified version of www.citec.fi
Comment 79 Johnny Stenback (:jst, jst@mozilla.com) 2000-04-18 15:37:38 PDT
This one's back, se the new attachment. Reopening.
Comment 80 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-04-18 15:43:10 PDT
jst - Are you sure you're not seeing bug 36080 (transitional DTD triggers strict
mode) instead?
Comment 81 Johnny Stenback (:jst, jst@mozilla.com) 2000-04-18 17:09:23 PDT
Duh, that's exactly what I'm seeing, thank's for bringing this to my attention.

Changing back to fixed. Sorry for the spam.
Comment 82 Johnny Stenback (:jst, jst@mozilla.com) 2000-04-18 17:10:08 PDT
... and verified.

Note You need to log in before you can comment on or make changes to this bug.