Closed Bug 178430 Opened 22 years ago Closed 21 years ago

table cell in middle column too narrow

Categories

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

x86
Windows XP
defect

Tracking

()

RESOLVED WONTFIX
mozilla1.3beta

People

(Reporter: jarrett, Assigned: karnaze)

References

()

Details

(Whiteboard: [PATCH])

Attachments

(6 files, 2 obsolete files)

I've seen this on some other pages also. IE and Opera display these pages
correctly. It seems it is not correctly formating some tables.

Problem in Mozilla 1.0, 1.1, 1.2b and Phoenix .4

jarrett
To tables.  The cell separator there has width="60" and we only make it 4px wide.
Assignee: jst → table
Status: UNCONFIRMED → NEW
Component: DOM HTML → Layout: Tables
Ever confirmed: true
QA Contact: stummala → amar
Summary: Formatting Engine is not correctly displaying → table cell in middle column too narrow
Attached file reduced test case (obsolete) —
Attached file reduced test case
Attachment #105247 - Attachment is obsolete: true
in the reduced testcase, table has fixed width as 500, 1st td has fixed width as
100, 2nd td has fixed width as 400, but 2nd's mes is bigger than 500.
so when balance column width, moz set 2nd width as its mes, and reduce first td.

But IE will enlarge outside table to contain these two cell.

I dont think it is a bug.
I think the 2nd cell block's mes should not be as big as it is.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.3beta
Taking the bug, changing the component.
Assignee: table → karnaze
Status: ASSIGNED → NEW
Component: Layout: Tables → Layout: Floats
Attached file another test case
Before the patch, the max element size of lines containing/impacted-by floaters
was using the largest max-element-size of frames adjacent (and not including)
the floaters combined with the floaters. With the patch the max element size
uses the 1st non whitespace frame instead. This gives a smaller max element
size while still having something impacted by the floaters.
Attachment #105545 - Attachment is obsolete: true
Attachment #105643 - Flags: superreview?(kin)
Attachment #105643 - Flags: review?(alexsavulov)
Blocks: 102100
Status: NEW → ASSIGNED
Whiteboard: [PATCH]
Comment on attachment 105643 [details] [diff] [review]
revised patch to handle additional test case

Chris, conviced me about the new algorithm. We had an extensive talk about this
and the fact that in the specs this special case is not handled. Anyway do the
regression testing to see what issues might arise.
Attachment #105643 - Flags: review?(alexsavulov) → review+
Could you describe the logic changes in this patch?  (I've only read the
comments in the bug, and haven't looked at the patch.)  The brief comments above
seem very wrong to me.  There are a number of cases where we need the min-size
(aka mes) to be larger than it is now, not smaller.  Furthermore, any frame that
the floater could be adjacent to needs to matter, since at a different width it
might end up adjacent to that frame.  Additionally, the floater information
needs to affect all normal flow descendants of the block that establishes the
formatting context (i.e., the one with NS_BLOCK_SPACE_MGR SET).  Yes, we have
bugs here, but I'm not sure in what direction this patch is an attempt to take us.

See bug 156629 (especially, since I proposed a new solution there in bug 156629
comment 15), bug 132021, and bug 97383.
Actually, I see the point about how you want the min-width to be narrower in
this case (which we can do since we switched to advancing past floaters rather
than forcing something to be on every line).

However, I'm not sure how this patch accounts for things like floaters in the
middle of a block, etc.  I need to think about those other bugs again, but I
don't have time now.
With the patch the mes width is the width of the floaters plus the width of the
1st non whitespace frame on the 1st line containing or impacted by floaters. I
had thought of other algorithms (e.g. the widest frame on the 1st line, some
percentage of the floater widths together with the width of the frames, the
widest text until a wider non text is encountered, etc). These may sound a bit
FISHY but I think each would probably make our rendering of floaters overall
more compatible with IE without any spec violation that I am aware of. 

David, what are the urls/tests where we need to make mes width bigger (when you
get time)? 

The idea I described in comment 14 would mean changing
nsBlockFrame::ComputeLineMaxElementSize to use the maximum rather than the sum
(a simple -1/+3 change, reverting the third of the 4 changes in the patch in
attachment 48932 [details] [diff] [review] on bug 97383).  What that would do to the testcase on this bug
is force the wide image to be below the floating image.  Otherwise I would want
to do something like what I described in the bugs cited in comment 13, I think.

The bug on this issue that led to the current algorithm was bug 97383.

I think using the idea of the first line containing floaters is wrong because
what is on the line containing the floaters changes between reflows at different
sizes, but we want the max-element-size to be the same no matter what size we
reflow at.  (This is one of the problems with our current reflow architecture
where we attempt to compute the max-element-size and max-width at the same time
as reflow.)
Actually, I think the right way to do the change I described in comment 14 is an
approach like the one in bug 186593.
Based on the analysis of these issues in bug 186593, this particular issue
should be wontfix.  Note that the original testcase for the issue this bug
covers is attachment 105253 [details].  A modified version of that using a right floater
instead of a left floater is attachment 115498 [details].  Our new behavior (thanks to bug
186593) is consistent with WinIE for right floaters but not for left floaters. 
WinIE's behavior is quite buggy, and I don't think there's a need to emulate the
bugs in it in detail, but I think our new behavior is closest to the general
idea of its behavior.

Marking as wontfix.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Depends on: 186593
Resolution: --- → WONTFIX
Comment on attachment 105643 [details] [diff] [review]
revised patch to handle additional test case

removing review request
Attachment #105643 - Flags: superreview?(kin)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: