Closed Bug 540247 Opened 15 years ago Closed 15 years ago

Faviconize Tab extension causes painting weirdness on session restore

Categories

(Core :: Web Painting, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(1 file)

1. Install Faviconize Tab extension https://addons.mozilla.org/en-US/firefox/addon/3780 (use nightly tester tools to override compatibility) 2. Open enough tabs so that they don't all fit in the tab bar. 3. Faviconize 3 or so of the first tabs (right click faviconize). 4. Select the last tab. 5. Make sure "Show my tabs from last time" is turned on. 6. Exit Firefox and start it again. It looks like when the Faviconize Tab extension shrinks the width of those first few tabs, all the tabs shift to the left, but some of the old location of the tabs is not invalidated. Regression range using nightlies includes bug 526394 which seems like the likely cause.
It's enough to just faviconize a tab when there are enough tabs to cause scrolling the in tab bar to see this bug.
Timothy, do you want to dig into this? If not, I'll take it.
I can look into it.
The problematic invalidate seems to come from nsIFrame::Redraw (in nsBox.cpp). In that call we have |this| as Box(box)(-1)@0xb0762d00 {0,0,90600,1440} [state=80c01010] [content=0xb0774be0] [overflow=0,0,86760,1560] [sc=0xb0754d98]< and 90600-86760 = 64 pixels is the space at the end of the tab bar not occupied by any tab (caused by the shrinking of one tab followed by no scrolling of the tabbar that would normally happen if say a tab was removed).
nsXULScrollFrame::LayoutScrollArea gets the minsize of its scrolled frame and sets it to that size. Then it calls Layout on the scrolled frame, SprocketLayout handles this and it calls Redraw to invalidate the frame which defaults to using the overflow rect, the overflow rect hasn't been updated from when nsXULScrollFrame::LayoutScrollArea changed the bounds yet. When Layout is finished, EndLayout -> SyncLayout gets called, and this updates the overflow rect and then resizes the view for the frame if it has one (it did before bug 526394) to the overflow rect and this invalidates the difference between the old and new rects of the view. Without a view we don't get this invalidate hence the painting weirdness. In XUL Layout it seems that it is the parents job to set the size of a child and generate any invalidation needed. So I think we should get the overflow rect of the scrolled frame in nsXULScrollFrame::LayoutScrollArea before and after Layout and invalidate the difference. This fixes the problem for me.
Some other observations: the invalidation from nsViewManager::ResizeView goes straight to UpdateView and hence doesn't get passed to any MozAfterPaint listeners. I can create a reftest for this bug that exhibits the problem on screen, but because the shrinking of the scrolled frame causes a scroll position change to happen we get a bitblit scroll, and the reftest harness updates bitblitted rects using drawWindow (ie a full paint, not by copying them) the problem isn't visible in the canvas the reftest harness is using. If we passed the scroll position change in the MozAfterPaint event with "scroll copy" rects then the reftest harness could emulate the bitblitting.
Attached patch patch (deleted) — Splinter Review
I included the reftest even though it will always pass, maybe if the reftest harness gets updated to bitblit like in comment 6 it will actually test something.
Assignee: nobody → tnikkel
Attachment #423753 - Flags: review?(roc)
Filed bug 542450 for the second paragraph in comment 6.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
After I landed this I stumbled upon the fact that bug 543681 seems to fix this without the patch in this bug.
(In reply to comment #11) > After I landed this I stumbled upon the fact that bug 543681 seems to fix this > without the patch in this bug. Scratch that, bug 543681 fixes the simplified testcases that I had, but not the original bug.
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: