14782 - [dogfood] mousing over links causes massive reflows when status button crops

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Mike Pinkerton (not reading bugmail)]

as we discussed in my cube. turn on paint flashing

go to the bonsai link and mouse over the links in the bullet lists. these links
are long enough to make the titled button for the status bar crop. this causes
the window to repaint.

i think this is also the cause of why mousing over links at my.netscape is really
really slow.

this happens on all platforms.

Comment 1

[Mike Pinkerton (not reading bugmail)]

cc'ing shaver, as he and i were talking about this earlier.

Comment 2

[Mike Pinkerton (not reading bugmail)]

cc'ing shaver, as he and i were talking about this earlier.

Comment 3

[Mike Pinkerton (not reading bugmail)]

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

Comment 4

[Mike Pinkerton (not reading bugmail)]

taking this back. fix in hand.

Comment 5

[Mike Pinkerton (not reading bugmail)]

fixed.

Comment 6

[rubydoo123]

Claudius -- since this has to do with the the function of the window -- you get
it.

Comment 7

[Claudius Gayle]

reopening bug, per discussions with pinkerton. It seems we're still
repainting much more than we have to

Comment 8

[Claudius Gayle]

clearing fixed resolution

Comment 9

[Mike Pinkerton (not reading bugmail)]

fixed again, this time it was a problem in the view manager.

Comment 10

[Claudius Gayle]

I don't really think this is fixed - otherwise there is some other problem. pink and I will discuss.

Comment 11

[Mike Pinkerton (not reading bugmail)]

yeah, we've regressed alright. this f*ing sucks.

Comment 12

[Mike Pinkerton (not reading bugmail)]

you can easily see this when mousing over the "Module Owners" link in
mozilla.org. Mousing out of that link causes the content area and sidebar to
reflow, though I can't tell why. From that point on, it seems to want to continue
to reflow when mousing over links until you mouse over the "Milestone 10" link
(for example) at which point all is ok again.

It seems like we get into a state where everything causes a full reflow and then
mousing over other links (that don't appear to do any different) gets us back to
the happy place we started in.

Comment 13

[Mike Pinkerton (not reading bugmail)]

p1, m11, accepting, cc'ing evaughan.

Comment 14

[leger]

Putting on [PDT-] radar.

Comment 15

[Mike Shaver (:shaver emeritus)]

What does ``PDT-'' mean here?  What made people decide that this wasn't dogfood?

Comment 16

[Mike Pinkerton (not reading bugmail)]

cc'ing pav

Comment 17

[Mike Pinkerton (not reading bugmail)]

if i pull the status area out of the status bar this goes away, smells like
titledbutton/box bugs.

Comment 18

[Mike Pinkerton (not reading bugmail)]

reassigning to brendan since i'm going on vacation.

Comment 19

[Brendan Eich [:brendan]]

More of pink's bugs he dumped on me, I'm dumping on you!

/be

Comment 20

[Kevin McCluskey (gone)]

Moving to M12

Comment 21

[Kevin McCluskey (gone)]

The problem seems to be in FlowChildAt in nsBoxFrame.cpp.

nsresult
nsBoxFrame::FlowChildAt(nsIFrame* childFrame,
                     nsIPresContext* aPresContext,
                     nsHTMLReflowMetrics&     desiredSize,
                     const nsHTMLReflowState& aReflowState,
                     nsReflowStatus&          aStatus,
                     nsCalculatedBoxInfo&     aInfo,
                     PRBool& aRedraw,
                     nsString& aReason)

...

if (currentRect.width == calcWidth && currentRect.height == calcHeight) {
  shouldReflow = PR_FALSE;
}

The calcWidth varies (+ or -) 1 twip for the content area each time this method
is called. This forces the content are to be reflowed.

If the following HACK is used the reflows can be suppressed.

       if (currentRect.width == calcWidth && currentRect.height == calcHeight) {
         shouldReflow = PR_FALSE;
       }
       else {
         if ((currentRect.width - calcWidth == 1) ||
             (currentRect.width - calcWidth == -1) ||
             (currentRect.width - calcWidth == 2) ||
             (currentRect.width - calcWidth == -2)) {
               shouldReflow = PR_FALSE;
                printf("Reflow avoided\n");
             }
        }

I think the real problem is in nsBoxFrame::LayoutChildrenInRect(nsRect& size).
It uses a floating point value to apply a percentage to the boxes it is laying
out. I suspect the use the floating point is causing a round off error which
results in the +=1 twip variation in the calculated width.

Eric will be changing it so percentage values are expressed as integers instead
of floats. This may solve this bug.

Re-assigning to Eric.

Comment 22

[Kevin McCluskey (gone)]

This is a general bug which is exhibited whenever the status bar text is
changed. Changing the status bar text causes an incremental reflow which box
layout converts to a resize reflow for the content area if the calculated width
is not = to the current width of the content area.

Comment 23

[Kevin McCluskey (gone)]

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

Comment 24

[Eric Vaughan]

This is now fixed. It happened because the box layout was using floating point
arithmetic and because of roundoff things were sometimes off by 1 twip causing
weirdness. Now all calculations are done in integers.

Comment 25

[Paul MacQuiddy]

verified