see attached testcase and the debug log. especially the last lines text 026D3A84 d=3030,285 me=3030 caption 026D3BB0 d=3480,1200 me=1788 where the strange MES reduction takes place. VP 026CF040 d=9180,4470 VP 026CF040 r=1 a=9180,4470 c=9180,4470 cnt=851 block 026D3490 r=1 a=9180,UC c=8940,UC cnt=856 text 026D3678 r=0 a=8940,UC c=UC,UC cnt=857 text 026D3678 d=0,0 tblO 026D37C8 r=0 a=8940,UC c=0,0 cnt=858 caption 026D3BB0 r=0 a=UC,UC c=UC,UC cnt=859 text 026D3A84 r=0 a=UC,UC c=UC,UC cnt=860 text 026D3A84 d=3030,285 me=3030 caption 026D3BB0 d=3930,1200 me=3930 tbl 026D3994 r=0 a=8940,UC c=UC,UC cnt=861 rowG 026D3D54 r=0 a=UC,UC c=UC,UC cnt=862 row 026D3EE0 r=0 a=UC,UC c=UC,UC cnt=863 cell 026D40B4 r=0 a=UC,UC c=UC,UC cnt=864 block 026D4114 r=0 a=UC,UC c=UC,UC cnt=865 text 026D4254 r=0 a=UC,UC c=UC,UC cnt=866 text 026D4254 d=795,285 me=435 block 026D4114 d=795,300 me=435 cell 026D40B4 d=825,330 me=465 row 026D3EE0 d=UC,330 rowG 026D3D54 d=UC,330 colG 026D4380 r=0 a=UC,UC c=UC,UC cnt=867 col 026D44A8 r=0 a=0,0 c=UC,UC cnt=868 col 026D44A8 d=0,0 colG 026D4380 d=0,0 rowG 026D3D54 r=2 a=885,UC c=885,UC cnt=869 row 026D3EE0 r=2 a=885,UC c=885,UC cnt=870 cell 026D40B4 r=2 a=825,UC c=795,UC cnt=871 block 026D4114 r=2 a=795,UC c=795,UC cnt=872 block 026D4114 d=795,300 cell 026D40B4 d=825,330 row 026D3EE0 d=885,330 rowG 026D3D54 d=885,330 colG 026D4380 r=2 a=885,UC c=885,UC cnt=873 col 026D44A8 r=0 a=0,0 c=UC,UC cnt=874 col 026D44A8 d=0,0 colG 026D4380 d=0,0 tbl 026D3994 d=975,480 caption 026D3BB0 r=0 a=3930,UC c=UC,UC cnt=875 text 026D3A84 r=2 a=3030,UC c=UC,UC cnt=876 text 026D3A84 d=3030,285 caption 026D3BB0 d=3930,1200 tblO 026D37C8 d=3930,1680 text 026D4500 r=0 a=8940,UC c=UC,UC cnt=877 text 026D4500 d=0,0 block 026D3490 d=8940,1680 VP 026CF040 d=9180,4470 VP 026CF040 r=1 a=9180,4470 c=9180,4470 cnt=878 block 026D3490 r=1 a=9180,UC c=8940,UC cnt=883 tblO 026D37C8 r=1 a=8940,UC c=0,0 cnt=884 caption 026D3BB0 r=1 a=3930,UC c=888,UC cnt=885 VALUE 888 is not a whole pixel text 026D3A84 r=3 a=888,UC c=UC,UC cnt=886 VALUE 888 is not a whole pixel text 026D3A84 d=3030,285 me=3030 caption 026D3BB0 d=3480,1200 me=1788 VALUE 1788 is not a whole pixel tblO 026D37C8 d=3480,1680 block 026D3490 d=8940,1680 VP 026CF040 d=9180,4470

It looks perfectly fine to me. It's returning the right MES the first time, and then when the DOM sets style="width: 20%", it's returning that width. The text is supposed to overflow.

>The text is supposed to overflow. NO, it is not, because the NS_BLOCK_WRAP_SIZE flag is set. And this means the block can't shrink below the MES of the text inside.

I disagree. Strongly.

Why in the world do we have an incorrect answer this close to being the final result?

David, due to my limited knowledge of english I don't get the complete understanding what you mean. >I disagree. Strongly. I understand that you disagree, I dont understand, why. 1) If we have a NS_BLOCK_WRAP_SIZE set should the text overflow the box? 2) If the box is not allowed to be shrinked below the text-MES why should we not report a MES back that includes the text-MES and borderpadding? >Why in the world do we have an incorrect answer this close to being the final result? 3) Do you mean the patch fixes at the wrong place, this can easily be true I would not argue against, it was more intended as a demonstration what looks wrong to me. 4)But this is block and line layout and you are the expert. The only thing I see as a consumer of the block code is that it reports back wrong sizes. Or my understanding of the meanings of the flags is wrong (remember my missunderstanding of unit_null). I gathered my knowledge from http://www.mozilla.org/newlayout/doc/block-and-line.html NS_BLOCK_WRAP_SIZE When this flag is set, the frame is supposed to ``wrap around'' all its in-flow children. This means that we may need to enlarge it to fit around any frames that stick outside its box after ... ? This is set by default for select frames, area frames, and table cell inner frames when the frame is created.

A width is a width. It can't be expanded. In other words, I think NS_BLOCK_WRAP_SIZE shouldn't exist as something separate from NS_BLOCK_SHRINK_WRAP. (Can you find the reason they were added as separate things? Are there testcases on the bug?)

(My suspicion was that it was added because, in a case in the block code where we are correct and MSIE is buggy, someone decided that blocks inside tables should be buggy like MSIE (for lack of reading/understanding the standards, perhaps?) so we had to have a separate way of handling blocks inside of tables.)

The code at http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsBlockFrame.cpp#1500 is indeed evil, removing the stuff fixes bug 46983 .

Agreed, it is evil. We must change it to not rely on the overflow area. It may be as simple as ditching the X case and fixing the Y case to just include the floater bottom edge.

I think I would prefer to see the cases that actually rely on this behaviour by placing a assertion and running the regression tests. My plan would be to remove the NS_BLOCK_WRAP_SIZE from the inner table cell frame and caption frame. Once this is done successfully I would leave it for David to kill this stuff entirely.

Retitling bug to reflect what I think it now means. Retitle again if you disagree. (Changing from "nsBlockFrame::ComputeFinalSize returns wrong MES" to "remove NS_BLOCK_WRAP_SIZE".)

This removes the implementation of NS_BLOCK_WRAP_SIZE and replaces it with what I think should be there. I haven't tested it, and I can't find where Bernd pointed out a testcase that broke with the more simple removal...

The patch causes problems with this testcase (simplified from layout/html/tests/table/bugs/bug4520.html). It also seems to cause a very slight reduction in table height in certain cases, although the only visible changes involve HRs at the end of table cells. I'm not quite sure I understand that and there's a possibility it could be some form of common noise on the regression tests.

The problems may be related to the case where |okToAddRectRgn| is false in nsBlockReflowState::FlowAndPlaceFloater. I think it would be nice if that did the x-offset calculation more the way nsBlockReflowContext::ReflowBlock does it for what seems at first glance to be the same thing. (Why do we need both pieces of code? What's the difference?) That is, attachment 99893 [details] [diff] [review] might help (see bug 159363).

(This would be so much easier if we didn't attempt to do min-width, pref-width, and final layout in the same functions.)

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

Re: comment #20, why don't we separate those computations? Is there a bug for that design flaw fix? /be

carrying over keyword foo and url from bug 145141.

Re comment 22: When I refer to something such as "cleaning up reflow", that's what I mean. It's probably a multiple people on a branch for a few weeks/months type of job unless we can find a way to do it incrementally, and either way we'd need to have a good bit of design discussion beforehand.

I suspect what I need to fix is the right floater code in nsBlockFrame::PlaceLine (perhaps in addition to what I mentioned in comment 19).

Well, there seem to be two problems: 1) in ComputeFinalSize, the code that adds in the bottom padding to the autoHeight is before the code that accounts for floats. Switching them helps many of the problems above. 2) This patch makes it so right floats aren't accounted for in the height calculation for one of the many shrinkwrapping concepts.

This patch is only failing one test that I don't think it should be failing. (I modified it a bit since I last ran the tests to avoid some asserts on the failing test, but I don't think those modifications should have changed the behavior on the other tests.) The one test is: http://lxr.mozilla.org/mozilla/source/layout/html/tests/table/bugs/bug7113.html and the problem only occurs on the first layout (not reflows caused by text zoom).

Regression test changes are the following: http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/28811.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug131020.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug131020-2.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug137388-1.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug137388-2.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug137388-3.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug4576.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug6674.html Inner table cell frame (block) narrower, but the table and the content of the cell are the same size. (Case of horizontal overflow from cell.) In other words, no visible changes. http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/81776.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug16252.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug17548.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug33137.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug60992.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/printing/bug138349-1.html Table cell containing HR is slightly shorter (less space below HR, due to weird vertical alignment on generated content "\n"). http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/87543.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/169620.htmlhttp://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug1055-2.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug12012.xul http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug32205-2.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug83786.html Table supposedly slightly shorter, but nothing visible. http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug7113.html Slight increase in table width on initial load, fixing an incremental reflow bug. *http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/printing/bug92215.html Alternative patch #1 breaks this one, by causing the lower float to have its left edge adjacent with the right edge of the upper float, instead of its right edge. (I forgot to check if the problem goes away on text zoom.) *http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/printing/163614.html Alternative patch #2 breaks this one (and causes an incremental reflow bug), by making the table wider (since at large widths the image near the bottom could overlap the floating images). I prefer alternative patch #2 since it should make right floats affect things the same way left floats do. Each alternative causes one regression in the regression tests. I'm not too concerned about taking either since I'm hoping we can rewrite all the min/max width stuff soon anyway (so that it doesn't get computed within |Reflow|).

Comment on attachment 109592 [details] [diff] [review] patch, alternative #2 r+sr=roc+moz I really like this approach to handling right floaters. The old approach always made me queasy. "infinity - x" ... ugh.

Fix checked in to trunk, 2002-12-18 16:11/12 PDT.

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