Tab marker for newly opened tabs isn't painted with Tree Style Tabs installed

NEW
Assigned to

Status

()

Core
DOM: Animation
P3
normal
a year ago
8 months ago

People

(Reporter: njn, Assigned: hiro)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
Created attachment 8800582 [details]
screenshot of missing tab marker

Steps to reproduce:

- Install the Tree Style Tab extension (https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/).

- Open a new tab by middle-clicking on a link.

- The tab marker for the tab doesn't paint. It eventually will paint when you move the mouse over it, or switch away from the window and back, or anything else that triggers a paint.

I've attached a screenshot. You can see the missing tab marker below the MetaFilter tab. The problem reproduces on both Mac and Linux.

I bisected this down to part 6 of bug 1304922. Hiro, can you please take a look?
Flags: needinfo?
I might have been missing (a) condition(s) that cause cascading change.
Assignee: nobody → hiikezoe
Flags: needinfo?
This seems to be a case that we need to request restyle for *animations* level even if there is no animations level effect sine the extension has only transitions. Odd..
Confirmed environment to reproduce this problem for me:

 * Nightly 52.0a1 20161012143919
 * Tree Style Tab 0.18.2016090802 (released version at AMO: https://addons.mozilla.org/ja/firefox/addon/tree-style-tab/)

I've noticed that this problem happens by these reasons:

 * TST applies "transition" for tabs' opacity.
 * Initially TST sets the opacity of a new tab to "0".
 * And, after a delay TST sets it to "1". Then the animation starts.
 * After 0.15 sec, TST resets the opacity of a tab with "tab.style.opacity = ''".
   By the CSS spec, the rendering engine will ignore invalid value and apply
   another (valid) value declared in CSS files or the default value.
   It is "1" for "opacity".

Then, no one (CSS files of Firefox itself or TST) declares "opacity:0", however window.getComputedStyle(tab).opacity reports a value "0". It should report "1".
I tried to minimize the list of conditions to reproduce this problem, but currently
I still don't know what is the core condition.
Thanks!

I sill don't understand what's going on there.  But as far as I can tell, there is no opacity transition from 0 to 1.
What I can see on debugger when I open a new tab on background;

First
 margin-top: 0px to -29px 150ms duration
 opacity: 0.5 to 0 150ms duration

550ms after
 margin-top: -29px to 0px 150ms duration
 opacity: 0 to 0.5 150ms duration <-- this transition is created at the same time of the margin-top transition above but at the next moment when I see the below transition, this opacity transition is disappeared.
 margin-left: 0px to 12px 200ms duration
 max-width: 65000x to 65250px 150ms duration
 min-width: 0.1px to 1px 150ms duration

I might be seeing a different element though since I am just checking transitions on "tabbrowser-tab" class.
(In reply to YUKI "Piro" Hiroshi from comment #3)
> Confirmed environment to reproduce this problem for me:
> 
>  * Nightly 52.0a1 20161012143919
>  * Tree Style Tab 0.18.2016090802 (released version at AMO:
> https://addons.mozilla.org/ja/firefox/addon/tree-style-tab/)
> 
> I've noticed that this problem happens by these reasons:
> 
>  * TST applies "transition" for tabs' opacity.
>  * Initially TST sets the opacity of a new tab to "0".
>  * And, after a delay TST sets it to "1". Then the animation starts.
>  * After 0.15 sec, TST resets the opacity of a tab with "tab.style.opacity =
> ''".
>    By the CSS spec, the rendering engine will ignore invalid value and apply
>    another (valid) value declared in CSS files or the default value.
>    It is "1" for "opacity".
> 
> Then, no one (CSS files of Firefox itself or TST) declares "opacity:0",
> however window.getComputedStyle(tab).opacity reports a value "0". It should
> report "1".

What about the getComputedStyle value on nightly prior to bug 1304922?
Flags: needinfo?(yuki)
The scenario:

 1. Ctrl-Shift-J to open the error console.
 2. Run "var t = gBrowser.addTab()"
 3. Wait for a while until the animation finishes.
 4. Run "window.getComputedStyle(t).opacity"

This problem doesn't happen on:
http://ftp.mozilla.org/pub/firefox/nightly/2016/10/2016-10-05-03-02-11-mozilla-central/firefox-52.0a1.en-US.win64.zip
window.getComputedStyle(t).opacity says "1".

But happens on:
http://ftp.mozilla.org/pub/firefox/nightly/2016/10/2016-10-06-03-02-08-mozilla-central/firefox-52.0a1.en-US.win64.zip
window.getComputedStyle(t).opacity says "0".
Flags: needinfo?(yuki)
I did dig into what's going on when a new tab is opened

1. the extension calls setProperty('opacity', 0, 'important')
2. after 280ms later (I don't know where this timeout comes from), the extension calls setProperty('opacity', '', 'important')
 2.1. as a result, at this moment, opacity: 0.5 is used[1], then an opacity transition is created.
3. at the same time of 2 (I mean in the same frame), firefox calls setAttribute('fadein', true)
 3.1. as a result, at this moment, opacity: 0.5 is still used because I guess it set by setProperty with important on 2 and the extension has no opacity style when fadein is true[2]
 3.2. as a result, the opacity transition is cancelled at [3] because start and end values are the same[4]. (I am not sure this is the right behavior but this is not related to bug 1304922)

[1] https://github.com/piroor/treestyletab/blob/master/content/treestyletab/treestyletab.css#L270
[2] https://github.com/piroor/treestyletab/blob/master/content/treestyletab/treestyletab.css#L273
[3] http://hg.mozilla.org/mozilla-central/file/2a1297e90c59/layout/style/nsTransitionManager.cpp#l697
[4] http://hg.mozilla.org/mozilla-central/file/2a1297e90c59/layout/style/nsTransitionManager.cpp#l639

What I can tell from the perspective of the extension, I think "opacity: 1 !important" is needed into [2].  Also I can't find places that 'opacity: 1' is set for tabbrowser-tab in the extension code.  So what I don't quite understand is who sets the 100% opacity in comment 3.
> What I can tell from the perspective of the extension, I think "opacity: 1 !important" is needed into [2].

"oapcity:1" is intentionally excluded from the declaration because someone (theme, userChrome.css, or other addons) can set different opacity for regular tabs.
The CSS spec says that the initial value of "opacity" is "1".
https://www.w3.org/TR/css3-color/#transparency
So, tabs should have "1" for its opacity from the initial value definition, when no one (Firefox itself, addons, and so on) sets valid value for "opacity".
Actually, after I move the cursor onto any chrome object, Gecko refreshes something and the invisible tab appears. At this moment, getComputedStyle reports its opacity as "1" - it seems to be resolved from the initial value.
In other words, when this problem appears, Gecko seems to forget the initial value of "opacity".
(In reply to YUKI "Piro" Hiroshi from comment #8)
> > What I can tell from the perspective of the extension, I think "opacity: 1 !important" is needed into [2].
> 
> "oapcity:1" is intentionally excluded from the declaration because someone
> (theme, userChrome.css, or other addons) can set different opacity for
> regular tabs.
> The CSS spec says that the initial value of "opacity" is "1".

Yes, I know. 

(In reply to YUKI "Piro" Hiroshi from comment #3)

>  * TST applies "transition" for tabs' opacity.
>  * Initially TST sets the opacity of a new tab to "0".
>  * And, after a delay TST sets it to "1". Then the animation starts.

My question is, you said 'TST sets', what is this trigger?
> My question is, you said 'TST sets', what is this trigger?

It is done here:
https://github.com/piroor/treestyletab/blob/master/modules/browser.js#L6865
To update some DOM attributes while the animation, TST executes the callback function multiple times and only the first time TST sets the opacity. So the animation is actually started on this time, not the time when "updateTabCollapsed()" is called.
And finally, after the animation is finished, TST sets opacity to '' (blank text, invalid value) here to forget the "opacity:1 !important" for the animation:
https://github.com/piroor/treestyletab/blob/master/modules/browser.js#L6889
(In reply to YUKI "Piro" Hiroshi from comment #10)

> https://github.com/piroor/treestyletab/blob/master/modules/browser.js#L6889

Interesting, this code is not included in the extension that I've installed.  I guess it has been changed by https://github.com/piroor/treestyletab/commit/0045f0e2eb67c07fb5b8f8891077e0e4574d4892.  So as I wrote in comment 7, at this moment 'fadein' is not yet applied, so the opacity value is 0.5.  I think *firefox' triggers the opacity 1.

Anyway, I am inclined to think that bug 1304922 revealed an existing bug.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #7)

> 3. at the same time of 2 (I mean in the same frame), firefox calls
> setAttribute('fadein', true)
>  3.1. as a result, at this moment, opacity: 0.5 is still used because I
> guess it set by setProperty with important on 2 and the extension has no
> opacity style when fadein is true[2]

The bug is here.  When firefox calls setAttribute('fadein', true), the opacity property should be 1.
> Interesting, this code is not included in the extension that I've installed. 

Oops, sorry. The XPI built from the latest code is available at:
http://piro.sakura.ne.jp/xul/xpi/nightly/
(Note: it is unsigned so you need to disable the signature check.)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #11)

> > 3. at the same time of 2 (I mean in the same frame), firefox calls
> > setAttribute('fadein', true)
> >  3.1. as a result, at this moment, opacity: 0.5 is still used because I
> > guess it set by setProperty with important on 2 and the extension has no
> > opacity style when fadein is true[2]
> 
> The bug is here.  When firefox calls setAttribute('fadein', true), the
> opacity property should be 1.

I did miss an important thing.  The opacity value in nsTransitionManager::ConsiderStartingTransition() after setting 'fadein' is 0 there.  I don't know why but it's the real problem.
This seems to be fixed on today's nightly.
Oops. Sorry. This issue is still there.
You need to log in before you can comment on or make changes to this bug.