29595 - [FLOAT] Text description overlapping right table on amazon.com

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Joseph Elwell]

The description text is overlapping the "Ready to Buy" table on the right. This
could be related to bug 24411 which was a floater bug that is now fixed.
Although this new bug only effects M15 builds apparently.

winNT build 2000022808 M14 nightly displays fine.
winNT build 2000022808 M15 nightly displays wrong.

I noticed that the build numbers are the same. So - I'm assuming this is stored
in a registry file? - I downloaded the M14 nightly from the directory :
2000-02-28-15-M14 whereas the M15 nightly was from : 2000-02-28-08-M15

Comment 1

[Joseph Elwell]

Some people on #mozillazine had problems reproducing this bug. Looking closer I 
found that these steps work on both M14 and M15 nightly builds:
1)close mozilla.
2)delete mozregistry.dat
3)launch mozilla.
4)minimize sidebar
5)load URL
*tadah*
apparently having the sidebar open fixes the problem.
Closing it and reloading brings the problem back.
reloading all day while it's broken doesn't help unless you open the sidebar.
I'm currently using winNT build 2000022808 M14 nightly

marking M14 and changing from beta2 to beta1, because this effects a top100 
site.

Comment 2

[Jim Roskind]

How severe is this?  Screen shot??  Work arounds?

Comment 3

[troy]

I'm pretty sure it isn't a new problem, and I think we already have an existing 
bug for an Amazon.com page. I couldn't find it when I looked

Comment 4

[troy]

It was bug #24411 that I was thinking of. It has been marked FIXED, although it 
doesn't completely look fixed to me. I see the text "Availability: Usually ships 
within 2" overlapping the blue "READY TO BUY?" form on the right.

Comment 5

[troy]

This case works fine for me. I haven't tried the specific set of 5 steps listed 
below, though.

I don't think we should hold up the beta for this since it doesn't seem to be as 
easily reproducible as bug #24411 was.

Reassigning to Buster since he is the one who fixed bug #24411

Comment 6

[troy]

I created a new bug report for the problem I saw in looking at the bug 24411 
page. It's bug 29618

Comment 7

[Joseph Elwell]

I filed bug 24411 myself. :)
However this bug (29595) is easy to reproduce. Steps 1 - 5 aren't necessary.
All you really need it:
1)have the sidebar.
2)have it minimized.
3)load URL.

I'm attaching a screenshot.
Current workaround (on winNT build 2000022824 M14 nightly) is to expand the sidebar.

Comment 8

[Joseph Elwell]

Attachment: screen capture of bug.

Comment 9

[buster]

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

Comment 10

[buster]

there seem to be several variants of this bug.  I will attach several test 
cases derived from the amazon page.

Comment 11

[buster]

Attachment: no tables, initial layout ok, resize max->min overlaps until text reaches windows edge

Comment 12

[buster]

Attachment: table containing floating divs: initial layout overlaps if window width is between (width of left float + width of text - width of right float) and (width of left float + width of text)

Comment 13

[Joseph Elwell]

Attachment: This is what the first test case did to my browser when playing with maximizing and minimizing the window!!

Comment 14

[buster]

by "min and max" I meant "small and large width", not the min/max window button.  
sorry for the poor choice of words.  the UI painting glitch you just attached is 
an enirely separate problem.

Comment 15

[buster]

major floater problem, probably solving this would solve several other layout 
errors on other important sites.  So I'm looking into it now.  Will report back 
with a risk assessment asap.

Comment 16

[Chris Nelson]

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

Comment 17

[leger]

Putting on PDT+ for beta1.  But will move to PDT- if not fixed by 03/03 or if 
this regresses.

Comment 18

[buster]

looks like we're over-optimizing the case where a line contains a block and is 
impacted by a right floater.  might be easy to correct this optimization.  still 
investigating...

Comment 19

[Jim Roskind]

Per whiteboard 3/3 transition date... and since I think we can go to beta1 
without this... I'm pushing this down to PDT- for beta1

Comment 20

[billw]

It looks like the web site
http://ad.doubleclick.net/ad/miningco.com/entertainment;svc=;site=hollywoodmovie;chan=entertainment;syn=about;pos=slot1;sz=468x60;ord=109532243168773403370762341
(this is a about.com site for a movie review - but look at any of their links
and most of them display the same behaviour)
have the same problem as the amazon site.  I'm not totally sure if it is the
same, but it looks related.

Comment 21

[buster]

fixed.  there are a bunch of similar bugs that I will mark as fixed, not dups, 
because they mostly have slightly different symptoms.

Basically, this fixes most cases where oddness happened specifically because a 
line was impacted by more than 1 floater.  There were 2 nasty, nasty problems: 

1) nsLineBox::SetLineIsImpactedByFloater(PRBool aValue) places aValue into a 
literal bit in a struct.  It used to be: 

  // mImpactedByFloater bit 
  void SetLineIsImpactedByFloater(PRBool aValue) { 
    mFlags.mImpactedByFloater = aValue; 
  } 
  struct FlagBits { 
    PRUint32 mDirty : 1; 
    PRUint32 mBlock : 1; 
    PRUint32 mImpactedByFloater : 1; 
    PRUint32 mTrimmed : 1; 
    PRUint32 mHasPercentageChild : 1; 
    PRUint32 mLineWrapped: 1; 

    PRUint32 mBreakType : 4; 

    PRUint32 mChildCount : 22; 
  }; 

However, there were places where Kipp was passing in the result of 
aState.IsImpactedByFloater(). 

  PRBool IsImpactedByFloater() { 
    return mBand.GetFloaterCount(); 
  } 

Finally, nsBlockBandData returns a PRInt32, not a PRBool. 

  PRInt32 GetFloaterCount() const { 
    return mLeftFloaters + mRightFloaters; 
  } 

There are cases where it might be useful to know the number of floaters 
impacting a line, so the method by itself is fine. But it's a deadly memory 
overwrite to pass in anything other than PR_TRUE or PR_FALSE to a method that 
sets a bit in a struct.  And we were passing in 2, 3, etc. 

Those flag bits are pretty important. This wreaked all kinds of havoc, as you 
can imagine.  And asaict, it doesn't show up in purify.  The current purify 
output is so noisy it's hard to tell, but I didn't see it. 

2) There were places where the current floater impact was compared against the 
previous notion of floater impact as booleans.  But as discussed above, there 
was a case where the boolean (0|1) was being compared against (0|positive int), 
so unless we were lucky and the new number of floaters impacting a line was 1, 
this test would fail. 

Comment 22

[Chris Petersen]

Fixed in the April 21 build.