373474 - Horizontal scrollbar buttons of page garbled when notification box comes up or disappears

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when opening this testcase the notification box of the browser comes up, because the page has an unknown plugin.
In current trunk builds this causes the bottom horizontal scrollbars of the page to become garbled. It also happens again when you click away the notification box.

This regressed between 2007-02-23 and 2007-02-24
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-02-23+03&maxdate=2007-02-24+06&cvsroot=%2Fcvsroot
Ït seems like a regression from bug 370379 to me.

Comment 1

[Martijn Wargers (dead)]

Attachment: Screenshot of what I see

Note that I use 26px wide scrollbar, it might be important to reproduce.

Comment 2

[Eli Friedman]

Attachment: Patch

I think this is right, although I'm not completely sure.  (This is a similar situation to abs. pos. frames, where getting rid of views also meant having to deal with invalidates properly.)

Comment 3

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

It's actually the caller of SetBounds that is responsible for invalidation if necessary. In this case I think the problematic caller is nsGfxScrollFrame::LayoutScrollbars via nsBoxFrame::LayoutChildAt. I think LayoutChildAt pushes invalidation responsibility up to parents --- or at least, should --- so we need to fix LayoutScrollbars here. And we should document these responsibilities in the appropriate header files...

BTW you can write those if statements a little more simply as "if (rect.TopLeft() != aRect.TopLeft())".

Comment 4

[Eli Friedman]

Attachment: Patch v2

Okay, I think this is right.

Comment 5

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

How about making a static helper LayoutAndInvalidate(nsBoxReflowState& aState, nsIBox* aBox, const nsRect& aRect) to share this code. Also, in there you could just do
  if (aBox->GetRect() != aRect) {
    nsRect r;
    r.Union(aRect, aBox->GetRect());
    aBox->GetParent()->Invalidate(r);
  }

Comment 6

[Eli Friedman]

Attachment: Patch v3

Slight problem with the previous version: it doesn't actually work!  This should be better.

Comment 7

[Eli Friedman]

(In reply to comment #6)
> Created an attachment (id=259835) [details]
> Patch v3
> 
> Slight problem with the previous version: it doesn't actually work!  This
> should be better.

Oops; that comment is about a revision I never actually posted.

Comment 8

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

Comment on attachment 259835 [details] [diff] [review]
Patch v3

+  //nsPoint oldPosition = aBox->GetPosition();
+  //nsRect oldRect = aBox->GetOverflowRect() + oldPosition;

Remove this

Comment 9

[Eli Friedman]

Checked in. (No test; we currently don't have any way to test invalidation bugs.)

Comment 10

[Jeff Walden [:Waldo]]

We'll figure out a way eventually, and we should keep track of this bug until then.