Last Comment Bug 469774 - Tab switcher leaves drawing turds on the UI
: Tab switcher leaves drawing turds on the UI
Status: VERIFIED FIXED
: verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Layout: View Rendering (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Markus Stange [:mstange]
:
:
Mentors:
https://bug463574.bugzilla.mozilla.or...
: 445404 463574 (view as bug list)
Depends on:
Blocks: 505404 465076
  Show dependency treegraph
 
Reported: 2008-12-15 18:34 PST by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2009-11-25 16:28 PST (History)
16 users (show)
dbaron: blocking1.9.2+
roc: wanted1.9.2+
hskupin: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed
.6-fixed


Attachments
v1 (4.26 KB, patch)
2009-09-06 20:38 PDT, Markus Stange [:mstange]
roc: review+
roc: approval1.9.2+
dveditz: approval1.9.1.6+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2008-12-15 18:34:24 PST
STEPS TO REPRODUCE:

1)  Open 9 tabs with http://web.mit.edu in them
2)  Open 1 tab with http://www.mozilla.org in it
3)  Position your Firefox window so that when the tab switcher comes up its
    search field is over your tabstrip (or really, any part of your chrome).
    This requires that the window not be maximized.
4)  Click the tab switcher button.
5)  Type "moz" in the search field in the tab switcher.
6)  Hit the Escape key twice.

ACTUAL RESULTS: Tab switcher is dismissed, but its search field is still show over the chrome.  Dragging the window off screen and back doesn't help.  Moving focus out of it does.

EXPECTED RESULTS: No turds left over when the tab switcher is dismissed.

ADDITIONAL INFORMATION: There's a bit of jitter when going from 2 pages to 1 and back in the switcher.  If that transition doesn't happen, the bug doesn't appear.
Comment 1 Nochum Sossonko [:Natch] 2008-12-15 18:48:36 PST
dupe of bug 463574? (see bug 461519 which was duped to said bug).
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2008-12-15 20:03:53 PST
Looks possible, though this one is a lot clearer in terms of reproducing.  I'll mark dependent for now, and figure that vlad and Dão will sort it out.
Comment 3 Dão Gottwald [:dao] 2008-12-16 00:26:57 PST
Yeah, this is better. Requesting blocking1.9.2 so that bug 463574 can be duped against this one...
Comment 4 Dão Gottwald [:dao] 2008-12-16 13:30:21 PST
see also bug 445404
Comment 5 Dão Gottwald [:dao] 2009-07-21 03:46:44 PDT
*** Bug 463574 has been marked as a duplicate of this bug. ***
Comment 6 Dão Gottwald [:dao] 2009-07-22 08:10:24 PDT
*** Bug 505658 has been marked as a duplicate of this bug. ***
Comment 7 Henrik Skupin (:whimboo) 2009-07-22 08:16:05 PDT
*** Bug 505749 has been marked as a duplicate of this bug. ***
Comment 8 Markus Stange [:mstange] 2009-07-22 09:44:21 PDT
Where is the code that prevents frames in a popup window from getting into the parent window's display lists?
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-28 03:08:47 PDT
Frames that have popup frame children just shouldn't traverse those children in their BuildDisplayList implementation.

As far as I can tell, that's true in nsPopupSetFrame.
Comment 10 Markus Stange [:mstange] 2009-07-28 03:20:49 PDT
I don't see where nsPopupSetFrame overrides BuildDisplayList.
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-28 03:31:24 PDT
nsBoxFrame::BuildDisplayList doesn't walk the popup list, though.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2009-07-30 12:53:07 PDT
Don't the steps-to-reproduce involve enabling the tab switcher?  How does one do that?
Comment 13 Dão Gottwald [:dao] 2009-07-30 12:56:31 PDT
The prefs are browser.ctrlTab.previews and browser.allTabs.previews.
Comment 14 Dão Gottwald [:dao] 2009-07-30 13:12:18 PDT
Only browser.allTabs.previews seems to be relevant here.

Unfortunately the STR are still more complicated...
If you have DOMi, select the browser window, find the element with the "allTabs-panel" id and add the class attribute with "KUI-panel" as the value. After that, see comment 0.
Comment 15 Dão Gottwald [:dao] 2009-08-02 05:54:12 PDT
(In reply to comment #14)
> Unfortunately the STR are still more complicated...
> If you have DOMi, select the browser window, find the element with the
> "allTabs-panel" id and add the class attribute with "KUI-panel" as the value.

Or uncomment line 260:
http://hg.mozilla.org/mozilla-central/annotate/94cd140316fb/browser/base/content/browser.xul#l257
Comment 16 Markus Stange [:mstange] 2009-09-06 01:49:30 PDT
(In reply to comment #11)
> nsBoxFrame::BuildDisplayList doesn't walk the popup list, though.

But the panel's placeholder frame is in the mFrames list of the popup set frame, and that gets replaced by the real panel frame in nsIFrame::BuildDisplayListForChild.
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-09-06 14:58:12 PDT
Hmm. Indeed! Then we should override BuildDisplayList in nsPopupSetFrame and have it do nothing.
Comment 18 Markus Stange [:mstange] 2009-09-06 20:38:28 PDT
Created attachment 399010 [details] [diff] [review]
v1

Overriding BuildDisplayList in nsPopupSetFrame won't be enough since popups aren't necessarily contained in a popupset. So I'm blocking them in nsIFrame::BuildDisplayListForChild.

GetType() == nsGkAtoms::menuPopupFrame seems to be the common way of identifying popup frames; I don't think we need a new IsPopup() method.

A little background info on this bug for those playing along at home:
The placeholder frames I mentioned in comment 16 are usually not harmful because they're 0x0 sized, so they don't intersect the dirty rect. That means that under normal circumstances they're not included in the display list -- except if NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO is set on them. And one thing that causes this flag to be set is the caret. So it was in fact the textbox in the panel that triggered this bug.
Comment 19 Markus Stange [:mstange] 2009-09-06 20:41:29 PDT
*** Bug 445404 has been marked as a duplicate of this bug. ***
Comment 20 Markus Stange [:mstange] 2009-09-07 15:50:50 PDT
http://hg.mozilla.org/mozilla-central/rev/f8ee121bd0e1
Comment 21 Markus Stange [:mstange] 2009-09-17 22:55:07 PDT
Comment on attachment 399010 [details] [diff] [review]
v1

Requesting branch approval: tiny, risk-free fix with test
Comment 22 Markus Stange [:mstange] 2009-09-21 19:45:24 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a8f78c8925ef
Comment 23 Henrik Skupin (:whimboo) 2009-09-22 14:16:59 PDT
Verified fixed on trunk and 1.9.2 with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20090922 Minefield/3.7a1pre ID:20090922030618

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20090922 Namoroka/3.6b1pre ID:20090922041132
Comment 24 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2009-09-23 13:38:44 PDT
Transferring blocking from bug 463574 (though already fixed).
Comment 25 Daniel Veditz [:dveditz] 2009-10-21 16:18:16 PDT
Comment on attachment 399010 [details] [diff] [review]
v1

Approved for 1.9.1.5, a=dveditz for release-drivers
Comment 26 Markus Stange [:mstange] 2009-10-28 12:32:10 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b67fcb2d3fe5
Comment 27 Al Billings [:abillings] 2009-11-25 16:23:26 PST
I can't repro the original problem on 1.9.1.5 (pre-fix) with Dao's control-tab addon (https://addons.mozilla.org/en-US/firefox/addon/5244) on Windows XP. Does this bug repro on 1.9.1 or are we just changing the code in all branches?
Comment 28 Al Billings [:abillings] 2009-11-25 16:28:38 PST
Speaking to Henrik, it turns out that he could only repro this on OS X. So I tried there and could repro it in 1.9.1.5 and see the fix with the nightly 1.9.1 build (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.6pre) Gecko/20091125 Shiretoko/3.5.6pre).

Note You need to log in before you can comment on or make changes to this bug.