940733 - When opening subviews in the panel menu, scroll bar flickers (appears, then disappears) during animation

Status: RESOLVED DUPLICATE of bug 1687828

(Firefox :: Menus, defect, P4)

Description

[Luke Wagner [:luke]]

This isn't a functional problem, but when I click the "history" button in the new Australis menu, during the animation where the history slides open, a scrollbar flickers visible then invisible.  This is a little visibly jarring and could potentially contribute to missed animation frames.

For reference, I'm using FF Nightly on Ubuntu.

Comment 1

[:Gijs (he/him)]

P4 for now, but it'd be higher if this is reproducible on other platforms.

Comment 2

[Luke Wagner [:luke]]

This also reproduces for me on Mac (although it is not as noticeable since the scrollbar fades in/out and overlays).

Comment 4

[Justin Dolske [:Dolske]]

Still a problem? Seems smooth to me.

Comment 5

[Alice0775 White]

I can still reproduce the problem
https://hg.mozilla.org/mozilla-central/rev/8122ffa9e1aa
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140306030201

Comment 6

[Luke Wagner [:luke]]

I can also reproduce the problem on recent Linux and Mac Nightly build.  It's a brief flash, but quite noticeable.

Comment 7

[:Gijs (he/him)]

Should this be higher prio if people can reproduce it? I haven't tried, also not sure how hard it is to reproduce and/or fix.

Comment 8

[Justin Dolske [:Dolske]]

Is it possible this is just the animation starting before Places has given us history results? It's one of those things where you're damned either way -- if you wait for results before starting the UI will feel sluggish/unresponsive, if you start before there are results you can get visual glitches like this.

How does the old history menu in the menubar feel for people who can reproduce this?

Oh, I do see the scrollbar show and then fadeout on OS X. But at comment 2 notes it's not very jarring due to the fadeout.

Alas I don't see the flicker either on Ubuntu (MattN's debug build in a VM).

I think P4 is about right for now, but would reconsider if it really looks worse than it sounds.

Comment 9

[Luke Wagner [:luke]]

I know approximately nothing about frontend development, but the problem seemed to be to be the following sequence:
 1. click History, the History pane starts sliding in from the right and the whole menu starts getting vertically taller
 2. the History pane is populated with the number entries that is expected to fit without scrolling once the pane is at its final vertical height
 3. since the animation in #1 isn't finished yet, the number of entries is bigger than the visible size and so we get a vertical scrollbar
 4. the animation finishes, the number of entries fits w/o any need for the scrollbar, so it goes away

Comment 10

[Justin Dolske [:Dolske]]

Maybe same issue as bug 977845?

Comment 11

[Marco Bonardo [:mak]]

(In reply to Justin Dolske [:Dolske] from comment #8)
> Is it possible this is just the animation starting before Places has given
> us history results?

The history widget doesn't use Places API, it does a direct async query.

> How does the old history menu in the menubar feel for people who can
> reproduce this?

The old menubar uses the synchronous API (exactly it uses a nsNavHistoryResult). That API is sometimes moving to async btw.

> Oh, I do see the scrollbar show and then fadeout on OS X. But at comment 2
> notes it's not very jarring due to the fadeout.

I see the same on Win8.1

Btw, I think it's more a frontend bug than a bug with results coming late, it should cope with asynchronous content since we are moving more and more stuff to async.

Comment 12

[u428464]

This is also the case with the developer subview for example and with the fact that the shortcuts are now shown in the menu it's quite more visible than before, because the shortcuts are briefly moved from the left to the right during the subview opening.

Comment 13

[:Gijs (he/him)]

As far as I know this is just because of the fact that we transition the size changes of the panel, and so for some time the panel view isn't high enough to accommodate all the items, so a scrollbar is shown.

I'm not really sure what could be done about this. We could set overflow: hidden, but that's likely to look just as bad...

Comment 14

[Luke Wagner [:luke]]

Using overflow: hidden sounds good; what would look wrong?  It seems like, for the very short time during the animation, some items would be clipped which is what I would expect to happen.  The only jarring thing is the scrollbar appearing and disappearing.

Comment 15

[Mike de Boer [:mikedeboer]]

I'd vote for overflow:hidden during the transition as well.

Comment 17

[Roman Müller]

I don't want to be annoying or something but is there any update on this issue? It's been like 5 months since this bug was reported.

It looks quite cheap in Australis menu and is as far as I know last graphical glitch I'm aware of.

Comment 18

[:Gijs (he/him)]

Attachment: set overflow:hidden during the transition,

Comment 19

[Mike de Boer [:mikedeboer]]

Comment on attachment 8406523 [details] [diff] [review]
set overflow:hidden during the transition,

Gijs, it might be me, but I don't see a difference when I open the Character Encoding subview on OSX with this patch applied... did you test it there, or only on Linux?

Comment 20

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #19)
> Comment on attachment 8406523 [details] [diff] [review]
> set overflow:hidden during the transition,
> 
> Gijs, it might be me, but I don't see a difference when I open the Character
> Encoding subview on OSX with this patch applied... did you test it there, or
> only on Linux?

I tested on OS X. No difference as in, there's still a scrollbar showing temporarily?

Comment 21

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs Kruitbosch from comment #20)
> I tested on OS X. No difference as in, there's still a scrollbar showing
> temporarily?

Yes, but now I see that the scrollbars might only be visible _after_ the transition, it's so fast :)
When I slow down the transition it actually looks like the patch works as advertised.

Comment 22

[Mike de Boer [:mikedeboer]]

But then again, when I slow down the transition and run it without the patch applied, I also don't see any scrollbars.

Comment 23

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #21)
> (In reply to :Gijs Kruitbosch from comment #20)
> > I tested on OS X. No difference as in, there's still a scrollbar showing
> > temporarily?
> 
> Yes, but now I see that the scrollbars might only be visible _after_ the
> transition, it's so fast :)
> When I slow down the transition it actually looks like the patch works as
> advertised.

I see. I tested by emptying out most of the panel and then opening the developer tools. There the scrollbar flash was sometimes visible, and now isn't anymore.

Comment 24

[Mike de Boer [:mikedeboer]]

Comment on attachment 8406523 [details] [diff] [review]
set overflow:hidden during the transition,

True. And I see a nice improvement on Linux and Win.

Thanks!

Comment 25

[:Gijs (he/him)]

Attachment: set overflow:hidden during the transition,

This seems to be enough to get rid of the last flash of scrollbar I was seeing on the history view on Linux.

Comment 26

[:Gijs (he/him)]

Comment on attachment 8406783 [details] [diff] [review]
set overflow:hidden during the transition,

These handlers are broken in some way. I don't really know why. But I'm going to figure it out and get a better patch, because while we keep the underlying brokenness we can't really fix this bug very well.

Comment 28

[u428464]

So bug 994194 seems to have fixed the history subview issue (side note the animation is now laggy but I'm not sure it's related), the issue is however still visible at least in the developer subview.

Comment 29

[:Gijs (he/him)]

(In reply to Guillaume C. [:ge3k0s] from comment #28)
> So bug 994194 seems to have fixed the history subview issue (side note the
> animation is now laggy but I'm not sure it's related), the issue is however
> still visible at least in the developer subview.

That patch only affected subviews opened as panels on their own from toolbars; It shouldn't have affected these cases at all. I think the history case is just intermittent and/or depends on how big the menu is, what OS you have, how many history items you have, and the phase of the moon.

:-(

I hope to get back to this bug this week, but I've been distracted by some other high-prio issues over the last few weeks.

Comment 30

[u428464]

(In reply to :Gijs Kruitbosch from comment #29)
> (In reply to Guillaume C. [:ge3k0s] from comment #28)
> > So bug 994194 seems to have fixed the history subview issue (side note the
> > animation is now laggy but I'm not sure it's related), the issue is however
> > still visible at least in the developer subview.
> 
> That patch only affected subviews opened as panels on their own from
> toolbars; It shouldn't have affected these cases at all. I think the history
> case is just intermittent and/or depends on how big the menu is, what OS you
> have, how many history items you have, and the phase of the moon.

Yeah that seems to be the case. It's strange that the scrollbar appears to flash only after the subview is extended. Although it's a minor issue I hope it'll get fixed. :-)

Comment 31

[Roman Müller]

I can confirm this issue still persists when opening subviews like History, Developer etc from appmenu. Scrollbar appears  for a brief moment during the animation.

Comment 32

[:Gijs (he/him)]

Making this bug's status reflect reality...

Comment 35

[Florian Quèze [:florian]]

Attachment: Screenshot of the flickering scrollbar on Mac

This still happens on currently nightlies, despite the rest of the animation now being smoother than ever.

Comment 36

[:Gijs (he/him)]

(In reply to Florian Quèze [:florian] [:flo] from comment #35)
> Created attachment 8879097 [details]
> Screenshot of the flickering scrollbar on Mac
> 
> This still happens on currently nightlies, despite the rest of the animation
> now being smoother than ever.

I think this is simply a consequence of the fact that, mid-animation, the panel is too small to contain some of the items, and so we have to display a scrollbar, even if the final state of the panel subview will be tall enough to accommodate all the items.

I think to avoid this we'd have to set overflow: hidden in both directions on the scrollable vbox that contains items in the panel. The issue is that this messes with measuring height/width, as well as not causing (too many) sync reflows, as well as with eventually having a scrollbar if/when we do need one in the end.

Comment 41

[:Paolo Amadini]

We should probably figure out whether a scroll bar is present when preparing the subview off-screen, make the scrollbar always visible or always hidden during the animation, and then restore the visibility to be automatic.

Comment 43

[:Gijs (he/him)]

This was recently fixed! \o/