469774 - Tab switcher leaves drawing turds on the UI

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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]]

dupe of bug 463574? (see bug 461519 which was duped to said bug).

Comment 2

[Boris Zbarsky [:bzbarsky]]

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]]

Yeah, this is better. Requesting blocking1.9.2 so that bug 463574 can be duped against this one...

Comment 4

[Dão Gottwald [:dao]]

see also bug 445404

Comment 8

[Markus Stange [:mstange]]

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)]

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]]

I don't see where nsPopupSetFrame overrides BuildDisplayList.

Comment 11

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

nsBoxFrame::BuildDisplayList doesn't walk the popup list, though.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Don't the steps-to-reproduce involve enabling the tab switcher?  How does one do that?

Comment 13

[Dão Gottwald [:dao]]

The prefs are browser.ctrlTab.previews and browser.allTabs.previews.

Comment 14

[Dão Gottwald [:dao]]

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]]

(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]]

(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)]

Hmm. Indeed! Then we should override BuildDisplayList in nsPopupSetFrame and have it do nothing.

Comment 18

[Markus Stange [:mstange]]

Attachment: 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 20

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/f8ee121bd0e1

Comment 21

[Markus Stange [:mstange]]

Comment on attachment 399010 [details] [diff] [review]
v1

Requesting branch approval: tiny, risk-free fix with test

Comment 22

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a8f78c8925ef

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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-4, no longer working on Mozilla)]

Transferring blocking from bug 463574 (though already fixed).

Comment 25

[Daniel Veditz [:dveditz]]

Comment on attachment 399010 [details] [diff] [review]
v1

Approved for 1.9.1.5, a=dveditz for release-drivers

Comment 26

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b67fcb2d3fe5

Comment 27

[Al Billings [:abillings - ex-MoCo]]

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 - ex-MoCo]]

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).