194726 - Major layout regressions in 1.3b 64-bit build

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Philip K. Warren]

Between Mozilla 1.3a and Mozilla 1.3b, layout of pages in the 64-bit build has
regressed greatly, to the point that current 64-bit builds are not usable. I
will attach screenshots of both before and after layout of a common site.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Any chance of a regression date range that's not quite so big?

Comment 2

[Philip K. Warren]

I'm downloading as many of the source tar images from the Mozilla FTP site as 
possible to try and track down when this starting occurring.

Comment 3

[Philip K. Warren]

This is present in a build from 2003-01-25. Will continue to narrow down builds 
which could be causing this.

Comment 4

[Philip K. Warren]

This behavior is not seen in a build from 01/15/2003 (14:00 GMT) and is seen in
a build from 01/20/2003 (16:00 GMT). I will continue to try and isolate this
down to a few fixes, but here are the Bonsai layout checkins during that time:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Flayout&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=01%2F15%2F2003+06%3A00%3A00&maxdate=01%2F20%2F2003+08%3A00%3A00&cvsroot=%2Fcvsroot

Comment 5

[Philip K. Warren]

Tracked it down even further, and found out it is happening between 01/18/2003-
01/19/2003. Here are the Bonsai checkins during this period:

http://makeashorterlink.com/?Z2FA13C93

I will continue to try and track this problem down further.

Comment 6

[Philip K. Warren]

Attachment: Screenshot

Comment 7

[Philip K. Warren]

I have confirmed that this behavior began with the checkin for Bug 141818.
Before applying this patch, layout looks fine. After applying it, I get behavior
like in the screenshot attached to this bug.

Comment 8

[Brendan Eich [:brendan]]

What size has long in your 64-bit compiler's type model?

/be

Comment 9

[Roland Mainz]

Brendan Eich wrote:
> What size has long in your 64-bit compiler's type model?

|long| and |long long| are usually 64bit on 64bit platforms (this includes AFAIK
both SPARCv9 and AIX 64bit).

Comment 10

[Philip K. Warren]

AIX 32-bit:
sizeof(long)=4
sizeof(long long)=8

AIX 64-bit:
sizeof(long)=8
sizeof(long long)=8

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

nsCellData.h has:

 union {
   nsTableCellFrame* mOrigCell;
   PRUint32          mBits;
 };

with the expectation that when mOrigCell is being used, the low bit of mBits
will be 0. But of course that's not necessarily so when mBits is only the first
half of mOrigCell.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

changing mBits to "unsigned long" should fix this.

Comment 13

[Brendan Eich [:brendan]]

unsigned long is fine -- PRUword would've been cool, but that type was
deprecated in NSPR2 (the NSPR1 type was pruword, but I'm not living in the past
:-).  From some bug I forget, we learned that long has to be pointer-sized in
any viable C type model.

/be

Comment 14

[Bernd]

Attachment: patch

from bug 141818
>I wouldn't worry about the 64-bit machines; I'm pretty sure PRUint32 will be
fine there.

I was worried ;-).

Comment 15

[Bernd]

this bug should block 1.3 as 64bit builds are useless with this bug

Comment 16

[Brendan Eich [:brendan]]

Comment on attachment 115713 [details] [diff] [review]
patch

How about a comment saying why unsigned long is the right type to overlay the
pointer arm of the union?

/be

Comment 17

[Bernd]

Attachment: revised patch

patch including brendans comments

Comment 18

[Philip K. Warren]

I can confirm that this patch does fix layout for 64-bit builds on AIX.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 115719 [details] [diff] [review]
revised patch

The comment should say "mBits must be an unsigned long because it must match
the size of mOrigCell on both 32- and 64-bit platforms"

given that, r+sr+a=roc+moz

Comment 20

[Bernd]

Attachment: patch with roberts comments

Comment 21

[Bernd]

timeless checked in a fix into branch removing the blocking sign

Comment 22

[Bernd]

fix checked in into trunk