At the test URL http://jim.pp.ru/covernews/2001-08-14_n61/2001-08-14_test.htm one can see the correct rendering of page. At the given bug URL http://jim.pp.ru/covernews/2001-08-14_n61/2001-08-14_05.htm layout is broken because the body-large table is missized (extended) in width. The only difference between the two is in the image that can be found by src="images/n610016.png". On broken page its attributes 'width' and 'height' are specified in source with correct numbers, and on normally looking page they aren't. The bug appears on Windows' builds after 0.9.4 (e.g. 2001092703) and doesn't appear on 0.9.4

a dup with the width and nowrap attributes again? It currently being looked at If remember.

I don't think so... There is no nowraps at all in these documents...

yup, sorry, missed that one, its not in the code anywhere.

confirm with mozilla 0.9.4 2001092603/win2k see also bug 102119 and http://bugscape/show_bug.cgi?id=5305 Beth, can we mark this topembed?

confirm with mozilla trunk build 2001092603/win2k.

Hunmn... is this a regression? If so, I believe many sites will present this. Adding here one example: http://mrshowbiz.go.com/movies/dvd/index.html

Another example: http://www.bu.edu/ Note that with these two examples, if you disable images, the layout is correct.

Adding Rafael and SOl. Rafael, I believe the above URL (The one you filed in the 99289 bug) is one case like this bug here.

Is this a regression?

the testcase from bu.edu site wfm in ns6.1rtm and mozilla 0.9.4 2001091303 on win2k but fails in mozilla 0.9.4 20010926 builds.

Can someone comment as to whether or not this is common case? Raf: Do you know if this is the same as the bug you filed earlier?

Changing platform to all. Tested and confirming with: WIn98 (0.9.4) Gecko/20010928

I am downloading and testing builds since the one Bob reported was good mozilla 0.9.4 2001091303. Will find out when the regression happened.

Ok, downloaded and testes with some 0.9.4 builds: Tested using the following testcases: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=51268 -------------------------------------------------------------- Win98;0.9.4 Gecko/20010917 - was working fineWin98;0.9.4 Gecko/20010918 - have the bug

I'm rowing against the procedural tide, but I'm nominating this nsbranch. We were asked to avoid regressions at all costs, and this looks a lot like a regression to me. A future commercial "Netscape" branded release shouldn't incorporate this regression, since it was not in Netscape 6.1

In addition to above comments, CC'ing jaimejr and chofmann.

Marc, may have a patch that fixed this.

My patch fixes the second case in the 3-testcase attachment since the whitespace between the images will make them wrap. However, the first testcase attachment does not appear to be related to the image wrapping bug that I fixed, and the other part of the 3-testcase attachment is still failing (table width ignored if the images are too wide). The rendering on http://mrshowbiz.go.com/movies/dvd/index.html is still broken with my patch, but http://www.bu.edu/ is fixed. I believe that there are a few different issues here, some fixed by my patch for bug 101674 and some not.

Agreed with Marc, we have different issues here. I added the MrShowBiz example without sufficient tests/testcase. My latest test using the testcases (URL and the ones I believe are related): ---------------------------------- Tests with: ------------ Win98, 0.9.4, Gecko/20010917 (1) [works] http://jim.pp.ru/covernews/2001-08-14_n61/2001-08-14_05.htm (2) [works] http://www.bu.edu/ (2.1) [works] http://bugzilla.mozilla.org/showattachment.cgi?attach_id=51268 (4) [works] http://bugzilla.mozilla.org/attachment.cgi?id=51243&action=view (testcase by Mats Palmgren possibly based in the URL) Tests with: ------------ win98, 0.9.4, Gecko/20010918 (1) [broken] http://jim.pp.ru/covernews/2001-08-14_n61/2001-08-14_05.htm (2) [broken] http://www.bu.edu/ (2.1) [broken] http://bugzilla.mozilla.org/showattachment.cgi?attach_id=51268 (4) [broken] http://bugzilla.mozilla.org/attachment.cgi?id=51243&action=view PS: (2.1) was based in (2)

Marking as PDT, and nsbranch+. This is an interesting bug. What are the chances that this will make the branch?

Lets keep this bug focused on the problem at the URL (http://jim.pp.ru/covernews/2001-08-14_n61/2001-08-14_05.htm) and the distilled testcase (attachment 51243 [details]) I made for it. The problem is that a /floating/ image makes the table grow beyond its /specified/ width. See bug 102216 for a similar case. http://mrshowbiz.go.com/movies/dvd/index.html is unrelated (I filed bug 102317 for this problem).

Marc, the first case in the "3 testcases" works fine in strict mode, so that is probably a dupe of bug 93396.

the reported problem with mrshowbiz that may be related to this bug is not the movies/dvd page but instead the http://mrshowbiz.go.com/news/index.html page.

This bug is getting confusing. So what is the test case? In the 1st attachment, the cell block is requesting a max elem size of 663 pixels (9945 twips) which is too large. I'm using the m094 branch and don't think this is a dup of bug 96228. Tbl 026E9D6C r=0 a=8940,UC c=0,0 cnt=0 Tbl 011946DC r=0 a=8940,UC c=8970,UC cnt=1 RowG 01194874 r=0 a=UC,UC c=UC,UC cnt=2 Row 026E9DB8 r=0 a=UC,UC c=UC,UC cnt=3 Cell 01194B70 r=0 a=UC,UC c=UC,UC cnt=4 Block 01194BCC r=0 a=UC,UC c=UC,UC cnt=5 Block 01194BCC d=48615,2025 me=9945 Cell 01194B70 d=48645,2055 me=9975 Row 026E9DB8 d=UC,2055 RowG 01194874 d=UC,2055

Reassigning to attinasi since it is related to bug 102216, but waterson may have some insight on what the problem is.

Doing a style-change reflow (i.e., by reducing, and then increasing text zoom) causes the page to lay out correctly. This is almost certainly due to the change I made to include the floater's MES in the line's MES. I'll take a look.

FWIW, I've reverted the change locally in my tree, and that will fix the problem with attachment 51243 [details]. It _does not_ fix the problem with http://mrshowbiz.go.com/news/index.html; let's get another bug on file for that.

Huh. During the unconstrained reflow, I think that we end with a situation something like this: |<------- 600px --------->| +-------------+ text text text text ... | First Image | +--------------+ +-------------+ | Second Image | +--------------+ |<-- 330px -->| |<-- 330px --->| |<----------- 660px ---------->| In other words, the first image is ``tall enough'' so that we include it in the MES computation of the paragraph containing the second image.

(Only the last hunk in attachment 51581 [details] [diff] [review] is relevant.)

dbaron, you were at the party in bug 97383; thoughts on the above?

This seems wrong. I'd think the real fix is to add the mes of only the first floater to the line's mes rather than that of all the ones in the band. The same problem could happen due to an mes calculation that happens during a constrained reflow (although perhaps in that case it would only be wrong in ways that wouldn't effect layout). Is that possible? Easy? (I think accounting for the minimum width (mes) and preferred width (max width) of blocks that contain floaters is probably the most difficult issue in CSS-based layout.)

Comment on attachment 51581 [details] [diff] [review] skanky way to fix this Wrong.

dbaron and I discussed computing the mes of a block as the mes of the floaters in the block's floater containing block plus the mes of the lines in the block. Attachment 51642 [details] [diff] does this, and as expected, does not fix the bug. I thought I'd attach it anyway, as its probably a lot simpler than the way we do things now.

what are the chances this will make the 094 branch?

Small to none.

thanks chris. is this a visible regression (i.e. is this something a user will be able to see or feel in great measure or frequency)?

Yes.

Attachment 51735 [details] [diff] attempts to fix this bug by only including floaters that appear on the current line in the MES. Unfortunately, this can underestimate the MES size when <br> elements are included in the block, and will regress bug 97383. I'll attach a test case that illustrates the problem.

I'm going to future this bug: 1) I think that the visual artifacts from over-estimating the MES are nowhere near as bad as the artifacts caused by under-estimating the MES. 2) It's not clear to me how we could otherwise efficiently account for floaters in the MES. If anyone has any ideas, I'd love to hear them.

(IOW, fixing this bug without regressing bug 97383 will be difficult, and IMO bug 97383 is more severe.)

PDT-, since this will not make the branch.

Just an idea ... What if you stop adding the widths of the floaters if they go beyond the specified width of the table (or cell as in bug 97383) ?

We're only including the MES (the minimum width, really) of a single floater.

OK, my point was - don't add it if the table/cell is constrained and it doesn't fit within the width (otherwise do).

But we need to add it in those cases to find out if the cell's contents won't fit in that constrained width. E.g., the cell <td width="200"><img width="300" align="left">Hello world.</td> must be wide enough to accomodate both the image and the wider of "Hello" and "world.".

cc'ing karnaze, who might have some ideas. mats, table reflow (as far as I understand it) is a two-pass affair. During the first pass, the table flows cells (really, just block frames) at an ``unconstrained width'', and requests that the block do a bit of extra work to compute the ``maximum element size'' (mes), which is really the ``minimum area required'' for the cell. For example, the width of the longest word. This allows the table to know the minimum width that it can flow the cell at. The ``preferred width'' of the cell are simply the boundaries of the block after the unconstrained reflow; most likely, a single long line. In this particular case, the result of this unconstrained reflow yields what my lovely ASCII art shows, above. Before fixing bug 97383, we used to report the block's mes as simply the maximum value of the individual mes's collected from each linebox. An individual linebox's mes computed as the maximum value of the mes's collected from the frames in the linebox, _or_ the maximum value of the mes's for any floated elements that impacted the line, whichever was larger. This leads to a situation where we break a ``de facto standard'' in float layout, which is that some text should always be placed next to the float. To fix that problem, we altered the linebox's mes computation to _add_ the mes of the line's frames to the mes of the floaters that impact the line. Which leads us to this bug. I've tried only accounting for the float's width in a line when the float's placeholder is in the current line (attachment 51735 [details] [diff] [review]); however, that's regressess 97383; e.g., for: <img align="right"><br>supercalifragilistic The <br> leaves the placeholder on the first line, and pushes the text to the next line, so we end up taking the maximum of _either_ the text's width or the image's width. Perhaps it is possible for the table to communicate information to the block frame that would indicate that it ought not consider the float's mes; however, I think that would lead to cases similar to bug 97383 where we under-estimate the mes in an over-constrained situation.

In the test case the floating img has a mes=330px, a bunch of text that has a mes=~20px, and a non floating img that has a mes=330px. Why can't the mes be ~350px instead of ~660px?

Because we don't know whether or not the non-floating image will be next to the floating image, and we can't know that until we lay it out. (The set of widths at which the contents of the cell can be laid out correctly may not be contiguous.)

karnaze: my comment on 2001-10-01 14:12 illustrates what is happening during the unconstrained reflow (and, therefore, why we come up with a 660px mes).

Begging your pardon... Is there any activity on this bug?

nominating because this is a regression.

removing myself from the cc list

chris k: see comment 52 for what's happening. We'll want to make sure we don't regress bug 97383 or its ilk when fixing. Thanks!

*** Bug 175269 has been marked as a duplicate of this bug. ***

nsbeta1+

nsbeta1-. Need to go after higher priority issues. Re-assigning to jkeiser.

After the checkin of bug 186593 I've dared to check this bug's URL. And... IT WORKS! Thank you Mr. Baron! Sorry my emotions - this bug was my primary :-). I'm not sure if it is really fixed with all the testcases, but the URL works as intended.

*** This bug has been marked as a duplicate of 186593 ***

*** Bug 120087 has been marked as a duplicate of this bug. ***