Closed Bug 581813 Opened 9 years ago Closed 9 years ago

Firefox with Tab Candy caused the Title and Tab Bars or the Menu and Tab Bars to appear black in colour

Categories

(Firefox Graveyard :: Panorama, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Ryuji, Assigned: Mardak)

References

Details

Attachments

(6 files, 4 obsolete files)

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
Blocks: 581748
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
Duplicate of this bug: 583240
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 582985
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.
QA Contact: general → tabcandy
Eek. This probably ought to block the beta release with tabcandy, looks really bad with the black and chunky window borders.
Blocks: 574217
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".
Assignee: nobody → ian
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?
Attached patch Like this? (obsolete) — Splinter Review
Attachment #464836 - Flags: review?
Attachment #464836 - Flags: review? → review?(ian)
Comment on attachment 464836 [details] [diff] [review]
Like this?

This patch doesn't even work!
Attachment #464836 - Flags: review?(ian) → review-
Attached patch Like this? (obsolete) — Splinter 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?
Attachment #464836 - Attachment is obsolete: true
Attachment #464901 - Flags: feedback?(ian)
Attached patch Or more like this? (obsolete) — Splinter Review
Tried to put the magic into UI.showTabView and UI.hideTabView.
Attachment #464901 - Attachment is obsolete: true
Attachment #464992 - Flags: feedback?(ian)
Attachment #464901 - Flags: feedback?(ian)
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.
Attached patch "like this" + "more like this" (obsolete) — Splinter Review
Attachment #464992 - Attachment is obsolete: true
Attachment #465023 - Flags: feedback?(ehsan)
Attachment #464992 - Flags: feedback?(ian)
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?
Duplicate of this bug: 581811
(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?
Attachment #465023 - Attachment is obsolete: true
Attachment #465023 - Flags: feedback?(ehsan)
Attachment #465253 - Flags: review?(ian)
Attachment #465253 - Flags: feedback?(fyan)
*evilcackle*
Assignee: ian → edilee
Blocks: 586679
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: 9 years ago
Resolution: --- → FIXED
Blocks: 586595
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
Attached image screenshot
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.
Attachment #465253 - Flags: review?(ian)
Attachment #465253 - Flags: feedback?(fryn)
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.