1088637 - Unnecessary vertical scrollbar appears when I move the mouse pointer over icon while PanelUI panel is animating

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Alice0775 White]

Attachment: screenshot

Steps To Reproduce:
1. Click  Hamburger button
2. Move the mouse cursor over icon while PanelUI panel is animating.
    (In other words, move mouse pointer to the downwards immediately after clicking the Hamburger button)

Actual Results:
Animation stops. And Unnecessary vertical scrollbar appears 

Expected Results:
No vertical scrollbar

Comment 1

[:Gijs (he/him)]

Is this a regression? And/or does it reproduce on current nightly?

Comment 2

[Alice0775 White]

(In reply to :Gijs Kruitbosch from comment #1)
> Is this a regression? And/or does it reproduce on current nightly?

Yes regression, because It does not happens on build of 2013/11/18 (Australis marge day)

Yes, It is reproducible on Latest Nightly(2014-10/28)

Comment 3

[Alice0775 White]

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f19ca5123d6a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0 ID:20140616112714
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e49b9b55cbb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0 ID:20140616114414
Pushlog:
Triggered by: Bug 994117

Comment 4

[Alice0775 White]

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f19ca5123d6a&tochange=9e49b9b55cbb

Comment 10

[Marco Castelluccio [:marco]]

Attachment: Screenshot 2

It can be even worse in some cases (for example when you know you need to click on an image in the bottom-left corner).

Comment 11

[Johan C]

Looks like this bug has been forgotten about. Bump :)

The bug is very easy to reproduce and it happens very frequently, even when you're not attempting to reproduce it.

Comment 13

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1088637 - check we get the right transition event, r?Enn

Bug 1088637 - check we get the right transition event, r?Enn

Comment 14

[:Gijs (he/him)]

Don't know why I didn't see this immediately after the regression window was posted, but here we are.

Comment 15

[Neil Deakin]

Comment on attachment 8648704 [details]
MozReview Request: Bug 1088637 - check we get the right transition event, r?Enn

I don't think aEvent->GetTarget() can fail, and the null-check of eventTarget is redundant with SameCOMIdentity.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/2534ceef7946

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2534ceef7946

Comment 18

[:Gijs (he/him)]

Alice and/or Johan, can you verify this is now fixed? I'm hoping to uplift to 41 and 42, and independent verification would help. Thanks!

Comment 19

[Alice0775 White]

I can verify this was fixed on latest Nightly43.0a1 (with disabled e10s and enabled e10s).

https://hg.mozilla.org/mozilla-central/rev/90d9b7c391d38ae118865bd87b5d011feee6dded
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0 ID:20150818030209

Comment 20

[:Gijs (he/him)]

(In reply to Alice0775 White from comment #19)
> I can verify this was fixed on latest Nightly43.0a1 (with disabled e10s and
> enabled e10s).
> 
> https://hg.mozilla.org/mozilla-central/rev/
> 90d9b7c391d38ae118865bd87b5d011feee6dded
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
> ID:20150818030209

Excellent, thanks!

Comment 21

[:Gijs (he/him)]

Comment on attachment 8648704 [details]
MozReview Request: Bug 1088637 - check we get the right transition event, r?Enn

Approval Request Comment
[Feature/regressing bug #]: bug 994117
[User impact if declined]: power users who are quick with their mice often see scrollbars in the main menupanel (and have the panel be too small). This could potentially affect the new control center usages as well
[Describe test coverage new/current, TreeHerder]: nope
[Risks and why]: very low-risk correctness fix to the transition listeners for popup animations, already independently verified
[String/UUID change made/needed]: nope

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8648704 [details]
MozReview Request: Bug 1088637 - check we get the right transition event, r?Enn

Given that the fix was verified, let's uplift to Aurora and Beta.

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b0b8104219f8
https://hg.mozilla.org/releases/mozilla-beta/rev/3aeb892a9bc5

Comment 24

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Attachment: 41b3_screenshot.png

I did some quick verification today on Windows 7 x64, using Firefox 41 Beta 3, and I can still easily reproduce the issue. As far as I can tell the fix should clearly be in this build. Note that the fix seems to indeed work just fine on Nightly.

Gijs, do you know of any reason why this fix would not work in 41 Beta 3?

Comment 25

[:Gijs (he/him)]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #24)
> Created attachment 8650901 [details]
> 41b3_screenshot.png
> 
> I did some quick verification today on Windows 7 x64, using Firefox 41 Beta
> 3, and I can still easily reproduce the issue. As far as I can tell the fix
> should clearly be in this build. Note that the fix seems to indeed work just
> fine on Nightly.
> 
> Gijs, do you know of any reason why this fix would not work in 41 Beta 3?

Not offhand, no. Does it work on Aurora?

Comment 26

[Alice0775 White]

I checked source files[1] of the 40.0b3.
And I confirmed that the 41.0b3 did not include the patch.

[1]https://ftp.mozilla.org/pub/mozilla.org/firefox/releases/41.0b3/source/firefox-41.0b3.source.tar.xz

(\firefox-41.0b3.source.tar.xz\firefox-41.0b3.source.tar\mozilla-beta\layout\xul\nsMenuPopupFrame.cpp)

Comment 27

[Alice0775 White]

oops.
> I checked source files[1] of the 40.0b3

I checked source files[1] of the 41.0b3

Comment 28

[:Gijs (he/him)]

Ugh, I should have been more careful in checking pushlog.

It seems like http://hg.mozilla.org/releases/mozilla-beta/rev/e393da8ff81b was used for tagging 41b3 (note the two kids) which caused this cset to miss b3. Verification should be possible in 41b4.

(see also: http://hg.mozilla.org/releases/mozilla-beta/graph which makes this more clear.)

Comment 29

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Thanks Gijs and Alice! We'll verify this on beta 4 or later then.

Comment 30

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 41 beta 3 (20150820142145) using Windows 7 64-bit.

Verified fixed on Firefox 41 Beta 4 (20150824144923), Firefox 43.0a1 (2015-08-25) and Firefox 42.0a2 (2015-08-25) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.

Comment 31

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336