Major layout regressions in 1.3b 64-bit build

RESOLVED FIXED

Status

()

Core
Layout
--
major
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Philip K. Warren, Unassigned)

Tracking

({64bit})

Trunk
Other
AIX
64bit
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed1.3)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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.
Any chance of a regression date range that's not quite so big?
(Reporter)

Comment 2

15 years ago
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.
(Reporter)

Comment 3

15 years ago
This is present in a build from 2003-01-25. Will continue to narrow down builds 
which could be causing this.
(Reporter)

Comment 4

15 years ago
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
(Reporter)

Comment 5

15 years ago
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.
(Reporter)

Comment 6

15 years ago
Created attachment 115670 [details]
Screenshot
(Reporter)

Comment 7

15 years ago
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.
What size has long in your 64-bit compiler's type model?

/be

Comment 9

15 years ago
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).
(Reporter)

Comment 10

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

AIX 64-bit:
sizeof(long)=8
sizeof(long long)=8
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.
changing mBits to "unsigned long" should fix this.
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

15 years ago
Created attachment 115713 [details] [diff] [review]
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 ;-).

Updated

15 years ago
Attachment #115713 - Flags: superreview?(roc+moz)
Attachment #115713 - Flags: review?(roc+moz)

Comment 15

15 years ago
this bug should block 1.3 as 64bit builds are useless with this bug
Flags: blocking1.3?
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

15 years ago
Created attachment 115719 [details] [diff] [review]
revised patch

patch including brendans comments
Attachment #115713 - Attachment is obsolete: true
(Reporter)

Comment 18

15 years ago
I can confirm that this patch does fix layout for 64-bit builds on AIX.
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
Attachment #115719 - Flags: superreview+
Attachment #115719 - Flags: review+
Attachment #115719 - Flags: approval1.3+
Flags: blocking1.3? → blocking1.3+
Attachment #115713 - Flags: superreview?(roc+moz)
Attachment #115713 - Flags: review?(roc+moz)

Comment 20

15 years ago
Created attachment 115766 [details] [diff] [review]
patch with roberts comments

Comment 21

15 years ago
timeless checked in a fix into branch removing the blocking sign
Flags: blocking1.3+

Comment 22

15 years ago
fix checked in into trunk
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Updated

15 years ago
Whiteboard: fixed1.3
You need to log in before you can comment on or make changes to this bug.