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

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Tan Wei Lin Jason [:Ryuji]]

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

Comment 1

[Tan Wei Lin Jason [:Ryuji]]

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.

Comment 2

[Tan Wei Lin Jason [:Ryuji]]

Attachment: Black Title and Tab bar when Firefox Menu is on

Comment 3

[Tan Wei Lin Jason [:Ryuji]]

Attachment: Black Menu and Tab Bar when Firefox Menu is off

Comment 6

[Tan Wei Lin Jason [:Ryuji]]

Attachment: Error in the latest build

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.

Comment 7

[Tan Wei Lin Jason [:Ryuji]]

Forgot to mention that if there is no changes in the error when the Firefox Button is turn on. 

Sorry for the spam.

Comment 8

[Justin Dolske [:Dolske]]

Attachment: not-fullscreen screenship

Eek. This probably ought to block the beta release with tabcandy, looks really bad with the black and chunky window borders.

Comment 9

[(no longer active)]

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

Comment 10

[Rob Arnold [:robarnold]]

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

Comment 11

[Ed Lee :Mardak]

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.

Comment 12

[Atul Varma [:atul]]

I've reproduced this bug on my Win7 VM, and the blackness does indeed only appear after triggering tabcandy.

Comment 13

[Atul Varma [:atul]]

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

Comment 14

[Aza Raskin [:aza]]

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

Comment 15

[Ian Gilman [:iangilman]]

Mardak suggests that visibility:hidden might be a better choice, if that works as well.

Comment 16

[Aza Raskin [:aza]]

I had that thought as well. Atul said that visibility:hidden didn't work, however.

Comment 17

[(no longer active)]

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

[(no longer active)]

Attachment: Like this?

Comment 19

[(no longer active)]

Comment on attachment 464836 [details] [diff] [review]
Like this?

This patch doesn't even work!

Comment 20

[(no longer active)]

Attachment: Like this?

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?

Comment 21

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Or more like this?

Tried to put the magic into UI.showTabView and UI.hideTabView.

Comment 22

[Ian Gilman [:iangilman]]

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.

Comment 23

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: "like this" + "more like this"

Comment 24

[Michael Yoshitaka Erlewine [:mitcho]]

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 25

[Ian Gilman [:iangilman]]

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?

Comment 27

[Michael Yoshitaka Erlewine [:mitcho]]

(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

Comment 28

[Michael Yoshitaka Erlewine [:mitcho]]

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.

Comment 29

[Michael Yoshitaka Erlewine [:mitcho]]

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

Comment 30

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: updated patch, still introduces geometry errors

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?

Comment 31

[Ed Lee :Mardak]

*evilcackle*

Comment 32

[Ed Lee :Mardak]

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.

Comment 33

[(no longer active)]

Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".

Comment 34

[Jim Jeffery not reading bug-mail 1/2/11]

This may have caused: https://bugzilla.mozilla.org/show_bug.cgi?id=587209

Comment 35

[pal-moz]

Attachment: screenshot

Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre ID:20100813041308

seems to be not fixed.

Comment 36

[pal-moz]

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 37

[Frank Yan (:fryn)]

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.