156629 - hyphen makes table width wrong

Status: VERIFIED DUPLICATE of bug 186593

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

Description

[CoL]

Checkin the url, the string: "labdarúgó-világbajnokságra", it is marked with red. 

Actual Result: 
this hyphen makes the first table (on the top) layout wrong. 
In the table bellow there is a space after "-", and in that case works fine. 
Mozilla build: 2002062713, win2k
Mozilla release 1.0 (2002053012)

Expected Results: there is a enough space, so mozilla should render it fine in
first case too, as mozilla does in older builds.
Works fine in: 
Netscape 6.2.3 (rv:0.9.4.1 Gecko/20020508 Netscape6/6.2.3)
Netscape 7.0 Preview Release 1 (rv:1.0rc2) Gecko/20020512 Netscape/7.0b1 )

Comment 1

[Boris Zbarsky [:bzbarsky]]

To tables.  Rendering is correct on first load, but reload makes the top table
bigger.  Shift-reload makes it fine again....

Comment 2

[CoL]

Hm, true, once it is ok, once it is wrong, shitf reload makes it good. But going
to url, or just reloading it is wrong. Magic? :)

Comment 3

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 4

[José Jeria]

I wonder if this is the same problem that is occuring in bug 146851. That bug is
about Aftonbladet.se, one of the most visited sites in Sweden.

Comment 5

[Mats Palmgren (inactive)]

I don't think it's related to aftonbladet.se (just a guess though :).
This reminds me more of "Testcase #4" in bug 120087.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It looks like we're not recalculating the max-element-size when the image load
comes in.  In particular, the max-element-size of blocks that have floaters is
the mes of the floater plus the mes of the inline contents.  (This should really
be done over a space-manager region, but I'm not sure if it is.)

It could be a tables bug, or it could be a bug dealing with mixing of
BRS_COMPUTEMAXELEMENTSIZE (ugh, it should be a separate GetMinSize method!) and
incremental reflow.

Comment 7

[Jerry]

Attachment: reflow log about the testcase

the first part of logfile is the reflow log of shift-reload, 
the second part of logfile is the reflow log of reload.

the key difference:
   when begin to reflow, both table's initial width are equal
tblO r=0 a=13944,UC c=0,0

   but at the end of reflow, 
first one get tblO d=3948,1470 
second one get tblO d=4844,1204

Comment 8

[Mats Palmgren (inactive)]

See also bug 156731.

Comment 9

[Jerry]

When I debug the testcase, I found that
shift-reload and reload follow different logic in nsImageFrame::GetDesiredSize
().

http://lxr.mozilla.org/mozilla/source/layout/html/base/src/nsImageFrame.cpp#782
if(con), do something, that's reloading page will go
if not, do someting, that's shift-reloading will go.
(since that code is related with bug 58646,
add attinasi to cc-list)

if I comment out 
http://lxr.mozilla.org/mozilla/source/layout/html/base/src/nsImageFrame.cpp#789,
shift-reload and reload will follow the same logic.
and to access that testcase, shift-reloading and reloading render samely.

Comment 10

[Jerry]

since it works with netscape 6.2, 
I think it is a regression.

maybe bug 58646 cause this regression.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The difference in the reload is probably based on whether the image is loaded
already.  The bug is presumably only triggered when the image loads after the
page has been displayed and we do an incremental reflow.  See comment 6.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch with comments explaining the problem

This patch has comments in the code that point out where I think the problem
is, but they don't propose a fix for the problem.  I think the problem lies in
the block code rather than the table code, so I think the component here is
incorrect.

What I suspect is happening (although I didn't actually verify it) is that, on
the incremental reflow, we're calculating the max-element-size with the lines
broken (this is a problem with calculating max-element-size (better called
minimum width) and maximum-width (better called preferred width) in the same
reflow process as doing layout).  However, in this case, we could actually fix
the problem by propagating the max-element-size for floaters and non-floaters
separately until we hit a block with NS_BLOCK_SPACE_MGR (note that there should
be an invariant that nothing cares about the max-element-size until we've hit a
block with NS_BLOCK_SPACE_MGR set).  The comments point out where the incorrect
code is, but they don't point out how to replace it, which I think is rather
complicated.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Note that I think the original testcase doesn't work anymore because we've
changed our line-breaking behavior for hyphens.)

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But, actually, there would need to be three values propagated, since we'd want
to propagate the minimum width value from specified widths on blocks separately,
and combine that with the previous value by maximum, rather than by addition.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

On second thoughts, separate propagation is both unnecessarily difficult and
unnecessarily strong.  There is a simpler and weaker solution that is still
sufficient to prevent overlap:  for each "inline" line, we should add to the mes
the largest mes of the floaters above it (i.e., already added) in the space
manager.  There are a number of ways to weaken this while still keeping it
sufficient, although I can't think of any that are easy to implement.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(See bug 120087 comment 40 (and bug 120087 comment 41) for some background that
helps explain my previous comments.)

Comment 17

[Jerry]

David, I have tested the testcase with 20020822 build.

it still have problem, shift-reload and reload give different result.

Comment 18

[Jerry]

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

Comment 19

[Jerry]

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

Comment 20

[Jerry]

assign to me

Comment 21

[Jerry]

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

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

See also bug 178430.

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that the algorithm I proposed in comment 15 requires a bit of care to
handle margin/border/padding correctly.  It's not that hard, though.  It just
requires tracking floaters on each side separately, using the total m/p/b
separating them from their side, and then using the current m/p/b on both sides
of the line as well as the max floater offsets above when combining the line and
floater numbers.

Comment 24

[David Tenser [:djst]]

Is this really Windows 2000 specific?

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 26

[Vedran Miletic]

v