Closed Bug 469774 Opened 11 years ago Closed 10 years ago
Tab switcher leaves drawing turds on the UI
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.
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.
Depends on: 463574
Yeah, this is better. Requesting blocking1.9.2 so that bug 463574 can be duped against this one...
Component: General → GFX
OS: Mac OS X → All
Product: Firefox → Core
QA Contact: general → general
Hardware: PC → All
see also bug 445404
Component: GFX → Layout: View Rendering
Product: Core Graveyard → Core
QA Contact: general → layout.view-rendering
No longer depends on: 463574
Duplicate of this bug: 463574
Where is the code that prevents frames in a popup window from getting into the parent window's display lists?
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.
I don't see where nsPopupSetFrame overrides BuildDisplayList.
nsBoxFrame::BuildDisplayList doesn't walk the popup list, though.
Don't the steps-to-reproduce involve enabling the tab switcher? How does one do that?
The prefs are browser.ctrlTab.previews and browser.allTabs.previews.
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.
Flags: blocking1.9.2? → wanted1.9.2+
(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
(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.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Hmm. Indeed! Then we should override BuildDisplayList in nsPopupSetFrame and have it do nothing.
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.
Attachment #399010 - Flags: review?(roc)
Attachment #399010 - Flags: review?(roc) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Comment on attachment 399010 [details] [diff] [review] v1 Requesting branch approval: tiny, risk-free fix with test
Attachment #399010 - Flags: approval22.214.171.124? → approval126.96.36.199?
Attachment #399010 - Flags: approval1.9.2? → approval1.9.2+
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
Status: RESOLVED → VERIFIED
Transferring blocking from bug 463574 (though already fixed).
Comment on attachment 399010 [details] [diff] [review] v1 Approved for 188.8.131.52, a=dveditz for release-drivers
Attachment #399010 - Flags: approval184.108.40.206? → approval220.127.116.11+
I can't repro the original problem on 18.104.22.168 (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?
Speaking to Henrik, it turns out that he could only repro this on OS X. So I tried there and could repro it in 22.214.171.124 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:126.96.36.199pre) Gecko/20091125 Shiretoko/3.5.6pre).
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.