Closed Bug 581813 Opened 11 years ago Closed 11 years ago
Firefox with Tab Candy caused the Title and Tab Bars or the Menu and Tab Bars to appear black in colour
164.60 KB, image/png
205.47 KB, image/png
150.55 KB, image/png
296.77 KB, image/png
4.56 KB, patch
|Details | Diff | Splinter Review|
34.78 KB, image/jpeg
User-Agent: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100725 Minefield/4.0b3pre Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100725 Minefield/4.0b3pre Tab Candy Integrated with Firefox caused problems such as the black appearance of Title and Tab Bars. Reproducible: Always
Component: General → TabCandy
Product: Firefox → Mozilla Labs
Target Milestone: --- → --
When the Menu bar is turn off and the Firefox Menu is visible, the title Bar and the tab Bar is black in colour. When the menu bar is turn on, the title bar is normal without any problems. Instead, the menu bar and Tab bar is now black in colour.
Summary: Tab Candy in Firefox caused Title and Tab Bars to appear Black in colour → Tab Candy in Firefox caused Title and Tab Bars or Menu and Tab Bars to appear Black in colour
Summary: Tab Candy in Firefox caused Title and Tab Bars or Menu and Tab Bars to appear Black in colour → Firefox with Tab Candy caused the Title and Tab Bars or the Menu and Tab Bars to appear black in colour
The Error still exist in the latest build which is Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre . If the Menu bar is on, the black error has somewhat changed.
Forgot to mention that if there is no changes in the error when the Firefox Button is turn on. Sorry for the spam.
Eek. This probably ought to block the beta release with tabcandy, looks really bad with the black and chunky window borders.
CCing some folks which might be familar with Aero stuff. It would be awesome if one of you guys can take a quick look here...
It would help if I knew what tab candy was doing to the browser. Last I looked it made its own invisible tab (which still shows up in the taskbar) and didn't properly switch the UI when going to/from that tab (kinda like print preview in that regard).
Right now it lives in an iframe that shares a parent xul:deck with the rest of the browser. We actually lazily create the iframe with the latest versions, so it might be interesting to see if the blackness appears only after triggering tabcandy.
I've reproduced this bug on my Win7 VM, and the blackness does indeed only appear after triggering tabcandy.
Yeah, it's definitely the iframe called "tab-view" that's the problem. If I go into DOMi and delete the iframe, the blackness goes away. Of course, tabcandy stops working too. :)
Atul also reports that using "display:none" on the iframe causes the blackness to go away. Thus a proposal for fixing this is: (1) When not in the Tab Candy interface, apply "display:none" to the iframe. (2) When in the Tab Candy interface, remove "display:none".
Mardak suggests that visibility:hidden might be a better choice, if that works as well.
I had that thought as well. Atul said that visibility:hidden didn't work, however.
(In reply to comment #15) > Mardak suggests that visibility:hidden might be a better choice, if that works > as well. What advantage would that give us?
Comment on attachment 464836 [details] [diff] [review] Like this? This patch doesn't even work!
Attachment #464836 - Flags: review?(ian) → review-
OK this patch works in the sense that it doesn't break tabcandy, but when you activate a tab in tab view by clicking on it, TabView.hide is not called. I don't know which function handles that. Maybe Ian can proceed from here?
Tried to put the magic into UI.showTabView and UI.hideTabView.
Comment on attachment 464992 [details] [diff] [review] Or more like this? Comments: * Looks like you've got some tab characters in there? Or just too many spaces? * We need to start off with the frame hidden, perhaps in TabView._initFrame, as well.
FYI all: I'm going to have to step away for the night. Fryn says that this patch does work, except that when you go back into tabcandy from a tab *the second time*, it leaves the iframe hidden = true, so it's black again on Windows. I think this patch is close. On the other hand, though, part of my frustration is that, repeatedly while testing this patch, I got some tabcandy assertions which look to be geometry related: an error during GroupItem construction, missing bounds, or being unable to compute styles with getComputedStyle. I can't imagine that the iframe actually being visible (or not visible) would affect this, so I think I need some sanity checking. If someone else could look at this patch and take the last extra step, that would be fantastic.
Comment on attachment 465023 [details] [diff] [review] "like this" + "more like this" The modifications to TabView.hide and .show are unnecessary at best, possibly causing problems at worst. The rest looks good, though. I'm worried, however, about the geometry issues you are seeing. Maybe .hidden is the wrong approach entirely?
(In reply to comment #25) > Comment on attachment 465023 [details] [diff] [review] > "like this" + "more like this" > > The modifications to TabView.hide and .show are unnecessary at best, possibly > causing problems at worst. The rest looks good, though. Removing the hidden = false in TabView.show makes it so tabview is not shown on the first launch. Perhaps this is a case where we should be triggering the hidden = false elsewhere from UI on load, instead, though? > I'm worried, however, about the geometry issues you are seeing. Maybe .hidden > is the wrong approach entirely? I just did a rebuild including the overnight m-c merge, with a new profile, and was able to reproduce this (with the patch above, or something close to it, of course). Here are the kinds of messages I get in the console when I open tabcandy: http://pastebin.mozilla.org/766630
Meant to reply to this too... (In reply to comment #25) > Maybe .hidden is the wrong approach entirely? Maybe? Maybe we can take the iframe's opacity to 0? Not understanding why I'm getting these geometry-related errors, however, apparently triggered by making the iframe hidden, it's hard for me to say for sure.
Might have found the root of a lot of this issue. For some reason we seem (with my patch, or something like it) to be adding an iframe for each invocation of tabview: http://img.skitch.com/20100812-x6jxydkrh9n1fmjtiqaf9yhyj1.jpg The first iframe there is the real deal. I went into a tab, then went into tabview again, and it created another iframe. This second (and subsequent) iframe then does *not* get the hidden = true attribute later, perhaps explaining why the blackout issue now is happening (on Windows) not on the first invocation of tabview, but the second. I'll keep digging and commenting here while y'all are asleep...
New patch: this patch no longer spawns multiple iframes, and I believe it sets the hidden property correctly (so the blackout on windows may be gone), but still introduces geometry issues for me. Please test?
Assignee: ian → edilee
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/874090a047df Push the TabView iframe down behind the tabbrowser when hiding TabView and restore the max height on show. This has slight issues in that it squishes the iframe contents as well as flashes black as we animate. Followup in bug 586679.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy. Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Version: unspecified → Trunk
This may have caused: https://bugzilla.mozilla.org/show_bug.cgi?id=587209
Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre ID:20100813041308 seems to be not fixed.
Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100814 Minefield/4.0b4pre ID:20100814040443 seems to be fixed by other bug checkin
Comment on attachment 465253 [details] [diff] [review] updated patch, still introduces geometry errors Since this has been fixed, I don't suppose we'll need feedback/review on this patch anymore.
You need to log in before you can comment on or make changes to this bug.