1.64 KB, image/gif
400 bytes, text/html
382 bytes, text/html
307 bytes, text/html
9.25 KB, patch
Alexandru Savulov: review+
|Details | Diff | Splinter Review|
401 bytes, text/html
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
Created attachment 105253 [details] 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
Created attachment 105545 [details] [diff] [review] patch to calculate a better max-element-width for a block containing floaters
Created attachment 105643 [details] [diff] [review] revised patch to handle additional 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
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.
Created attachment 115498 [details] attachment 105253 [details], except with right floater instead of left
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
Last Resolved: 15 years ago
Depends on: 186593
Resolution: --- → WONTFIX
Comment on attachment 105643 [details] [diff] [review] revised patch to handle additional test case removing review request
You need to log in before you can comment on or make changes to this bug.