92143 - resizing the browser window distorts the layout

Status: VERIFIED FIXED

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

Description

[Madhur Bhatia]

buildid: 20010724 window2000

1. go to http://www.hotmail.com
2. resize the window

actual:
you will notice that the layout of the page gets distorted. The right hand side 
table overlaps the left hand side table.
Please view the screen shot I have attached.

NOTE: This bug can be seen in neither Netscape 4.77 nor Netscape 6.01.
But I see this layout distortion problem right from Netscape 6.1 Preview 
release onwards.
This is will directly affect the user experience. Therefore should be looked 
into with the hightest urgency.

Comment 1

[Madhur Bhatia]

Attachment: screen shot

Comment 2

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 3

[Mats Palmgren (inactive)]

Adding kw testcase,top100 and -> HTMLTables

Comment 4

[Mats Palmgren (inactive)]

Seems to be a NOWRAP issue again.

Comment 5

[Mats Palmgren (inactive)]

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

Comment 6

[Katsuhiko Momoi]

Over to attinasi while karnaze is out.

Comment 7

[Madhur Bhatia]

adding keyword 'topembed' - corresponding bugscape bug 
http://bugscape.mcom.com/show_bug.cgi?id=7897.

I will be attaching another reduced testcase also.

Comment 8

[Madhur Bhatia]

Attachment: reduced testcase

Comment 9

[Christopher Hoess (gone)]

WFM, recent linux CVS build.

Comment 10

[Marc Attinasi]

The second testcase works fine, but the first testcase and the actual URL still
do not work. It looks like a cell with a specified width and NOWRAP is allowed
to shrink below the specified width, and is allowed to wrap as well (?). Same
problem on Linux ans Win2K

Looks like a table bug to me... cc'in Alex so he can check it out.

Comment 11

[Alexandru Savulov]

ressigning to me

Comment 12

[Alexandru Savulov]

when we rebalance the table after the initial reflow, we have to keep the fixed
width on a width cell even though the available width falls below the fixed
width of the cell.
that means that in quirks mode we have to check for nowrap on a fixed width cell
and change the way rebalancing column widths works (honour the width request) to
emulate the IE behaviour. (BTW: Opera does the same we do at the moment)

Comment 13

[Alexandru Savulov]

Attachment: proposed fix for hotmail.com (read following comment)

Comment 14

[Alexandru Savulov]

the proposed patch [attachment 49291 [details] [diff] [review]] fixes the URL hotmail.com.
 
in this patch in the case of a fixed width cell, I check for quirks mode and
then  I check the HTML content for the nowrap attribute. I cannot check the cell
frame be cause we don't map nowrap if there is a fixed width on the cell. one
may propose to map the nowrap so we don't have to check the content, but this
would result in displaying the text in a sigle line and this is not what the
effect of the pach is intended to be. IE/NAV use the nowrap attribute as a hint
for not resizing (shrinking) the cell bellow the fixed width size. 

the patch is not perfect be cause if i add a procentage cell on the same row, 

  <tr>
   <td width="312" nowrap>...</td>
   <td width="50%"></td>
  </tr>

this will cause the WIDTH="..." of the fixed width cell to be ignored. IE/NAV
keep the width of the fixed width cell (that also has nowrap on it) even then
when the adjacent cell has a procentage width on it while resizing.

anyway, with the proposed patch mozilla performs better than what we did before
in quirks mode and repairs a top100/topembed issue

need r/sr=

Comment 15

[Alexandru Savulov]

Attachment: moved patch from BasicTableLayoutStrategy to nsTableCellFrame

Comment 16

[Alexandru Savulov]

modified the first patch. moved to nsTableCellFrame as proposed by Chris Karnaze.

the new patch is testing for the following conditions to modify the min width
(max element size) of a cell:

- the cell has to have either width="[number]" or width: [number]px | cm | em...
- the cell does not have white-space: nowrap | pre
- the cell has nowrap in the tag (not mapped but still visible in the content)

then the max element size is set to the value of the fixed width.

need sr=/r=

Comment 17

[Marc Attinasi]

Comment on attachment 49958 [details] [diff] [review]
moved patch from BasicTableLayoutStrategy to nsTableCellFrame

You do not need to call get() on cellContent, just use the operator -> like cellContent->GetAttr(...)

check the result via NS_SUCCEEDED(result) instead of comparing to NS_OK.

Those two nits, and a review from karnaze and sr=attinasi

Comment 18

[karnaze (gone)]

r=karnaze, assumming it passes the table regression tests.

Comment 19

[Marc Attinasi]

Actually, check the documentation for GetAttr, it says:

 * <LI>If the attribute is not set and has no default value, return
 * NS_CONTENT_ATTR_NOT_THERE.
 * <LI>If the attribute exists, but has no value, return
 * NS_CONTENT_ATTR_NO_VALUE.
 * <LI>If the attribute has a non-empty value, set ret to
 * be the value, and return NS_CONTENT_ATTR_HAS_VALUE (== NS_OK).

So for NOWRAP, I think you need to check for NS_CONTENT_ATTR_NO_VALUE and for
NS_CONTENT_ATTR_HAS_VALUE. Please disregard what I said before about checking
for NS_SUCCESS, it is incorrect for this call.

Comment 20

[Alexandru Savulov]

Attachment: final patch with the modifications requested by Marc Attinasi

Comment 21

[Alexandru Savulov]

we (Marc and I) agreed subsequently that checking 

NS_CONTENT_ATTR_NOT_THERE != result

is sufficient in this case

Comment 22

[Alexandru Savulov]

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

Comment 23

[Alexandru Savulov]

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

Comment 24

[karnaze (gone)]

The patch has been checked in.

Comment 25

[Mats Palmgren (inactive)]

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

Comment 26

[Madhur Bhatia]

I can still see the bug on win2000 -- buildID : 2001-10-16-05-0.9.4 (branch 
build)

Comment 27

[Alexandru Savulov]

this needs PDT+ approval to be checked into the branch (work in progress)

Comment 28

[Jaime Rodriguez, Jr.]

was this present in Netscape 6.1?

Comment 29

[Jaime Rodriguez, Jr.]

was this present in Netscape 6.1?

Comment 30

[Alexandru Savulov]

yes, the bug is present in Netscape 6.1

Comment 31

[Kevin McCluskey (gone)]

amar: can you verify that this bug was fixed on the trunk? Thanks

Comment 32

[Amarendra Hanumanula]

 The checkins fixed the problem. verified on build ID : 2001103103- Trunk
Platforms: WIN2K and redhat linux 7.1

Comment 33

[Judson Valeski]

please land on the 0.9.4 branch and mark it w/ the "fixed0.9.4" keyword once
it's landed there.

Comment 34

[Alexandru Savulov]

fixed on branch 0.9.4 (+ fix for bug 101883 that had to go with this fix)

Comment 35

[Amarendra Hanumanula]

 Verified.. Fixed on branch 0.9.4ec build Id: 2002011320. Adding "verified0.9.4" 
keyword