981637 - "overflow" event sometimes doesn't fire until triggering hover/focus on "overflowing" animated transformed element

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[:Gijs (he/him)]

We're hitting this in Firefox's UI ([1], [2]), and HTML is also affected. A simplified testcase is here:

http://jsbin.com/yozum/2/edit

I can pull it all in a single HTML file if that's necessary.

Basically, it seems that if the element is focusable, and you click it while it is bigger than its container, it triggers an overflow event - but if you leave it alone for the duration of the animation, no event is triggered.

I'm not familiar with how transform and overflow events are meant to work, but I would think that this is a bug - either it should always cause an overflow event, or it should never do so.

David, I asked a bunch of folks on IRC over the past week, and the last person (smaug) shifted the buck to you - do you have any idea what the 'correct' behaviour is, and if there's any chance to get this fixed reasonably quickly?

[1] bug 884370
[2] bug 970013, bug 981548

Comment 1

[Markus Stange [:mstange]]

If I set outline:none on #animating, I don't get an overflow event on click any more. So it looks like it's not the focusing itself but rather the focus outline that causes the overflow event?

Comment 2

[:Gijs (he/him)]

(In reply to Markus Stange [:mstange] from comment #1)
> If I set outline:none on #animating, I don't get an overflow event on click
> any more. So it looks like it's not the focusing itself but rather the focus
> outline that causes the overflow event?

Quite possibly. In the Firefox UI that's at issue here there are :active and :hover styles on a bunch of things, and disabling all of them in such a way that there are no style changes when hovering/clicking while still allowing pointer-events is non-trivial. :-(

(setting pointer-events: none has the downside that people can't interact with the button, and because this is a XUL toolbarbutton with a dropmarker, that means either side of the button, which is even more problematic :-( )

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So I suspect what happened is that we added an optimization codepath in bug 524925 (since modified in various ways) that avoided doing Reflow when all we needed to do was update overflow areas.  The code for that optimization fails to fire overflow events in the case where ScrollFrameHelper::UpdateOverflow (previously nsGfxScrollFrameInner::UpdateOverflow) decides not to do reflow.

So basically I think what happened here is that bug 524925 broke overflow events on overflow:hidden containers when we use the nsChangeHint_UpdateOverflow optimization to handle style changes (such as for transforms).  That's why you're seeing inconsistent behavior with clicking, etc.; whenever you do something that defeats the optimization and actually causes reflow, you get the overflow events.  This can probably be fixed with a PostOverflowEvent call in ScrollFrameHelper::UpdateOverflow (after the "return false").

That doesn't agree with your expectation, I think -- you seem to want fewer overflow events here, yet the regression is actually in the opposite direction.


(Then, there's the question of when overflow events should fire.  They're documented in our event code as XUL events (at least they use EventNameType_XUL in nsEventNameList.h).  Is there a Web standard for them, or are they Mozilla-specific?  Either way -- for what sort of overflow should they fire?  scrollable overflow, visual overflow, or something else?  It probably makes more sense to use scrollable overflow, I think, given that they're associated with scrollable areas -- and also since that's already a Web-exposed concept, and visual overflow is not.  It *looks* like that's what they're doing, though I'm not sure.  So I don't immediately see anything wrong here, but I haven't looked that far yet.)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Make overflow events fire correctly in UpdateOverflow codepath

I confirmed that the mochitest fails without the patch and passes with
it, both standalone and in the directory harness.

Comment 5

[:Gijs (he/him)]

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-7) from comment #3)
> That doesn't agree with your expectation, I think -- you seem to want fewer
> overflow events here, yet the regression is actually in the opposite
> direction.

Right. If this bug gets fixed, we'll need to find another way to fix our UI issues, which will probably be tricky. :-(

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 8390214 [details] [diff] [review]
Make overflow events fire correctly in UpdateOverflow codepath

r=mats

Comment 7

[Mats Palmgren (inactive)]

Other UAs don't fire overflow events for ordinary HTML content AFAICT, so I think we should
restrict these events to privileged content and/or make a proposal for standardization.

Comment 8

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

I don't think we can do that.
See for example http://help.dottoro.com/ljbnnejo.php

Comment 9

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

er, I mean we can't restrict those events to chrome only.
So we need to spec the events.

Comment 10

[Mats Palmgren (inactive)]

I wasn't aware of the overflowchanged event in other UAs.  Then I agree there's a need to
standardize some variant of these events.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f44d993a20de

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f44d993a20de