1417036 - "Save to Pocket" drop down menu glitches

Status: VERIFIED FIXED

(Firefox :: Pocket, defect)

Description

[Emil Ghitta, QA [:emilghitta]]

Attachment: Pocket.gif

[Affected versions]:
59.0a1 (BuildId:20171113220112)
58.0b3 (BuildId:20171114032831)

[Unaffected versions]:
57.0 (BuildId:20171112125346)

[Affected platforms]:
Windows 10 64bit.
macOS 10.12.

[Unaffected platforms]:
Ubuntu 16.04 64bit.

[Steps to reproduce]:
1. Launch Firefox with a clean profile.
2. Click the "Save to Pocket" button.

[Expected result]:
The pocket menu is displayed without issues.

[Actual result]:
The pocket menu glitches.

[Regression range]:

This is a regression:

Last good revision: 6a3ca81dc56ab7687ca70b1b98fbe397aa3fa388
First bad revision: 9f83f552a6118f1061ee5afd87578d9b5b82b308

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6a3ca81dc56ab7687ca70b1b98fbe397aa3fa388&tochange=9f83f552a6118f1061ee5afd87578d9b5b82b308

[Additional notes]:
For further details regarding this issue please observe the attached screencast.

It seems that I couldn't reproduce this issue on Ubuntu 16.04.

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Drew!

Can you please have a look into this ? 

Thanks!

Comment 2

[Drew Willcoxon :adw]

What exactly is the glitch?

Comment 3

[Emil Ghitta, QA [:emilghitta]]

Attachment: Good.gif

The Pocket panel doesn't seem to open smoothly.

It looks like firstly it is opening an empty panel and then immediately corrects itself.

I attached how the panel opened on an unaffected build (57).

Comment 4

[Drew Willcoxon :adw]

I see, thanks for explaining further.  I can reproduce it on the current Nightly on macOS, too.

Comment 5

[Drew Willcoxon :adw]

Attachment: Bug 1417036 - "Save to Pocket" drop down menu glitches.

Review commit: https://reviewboard.mozilla.org/r/199878/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199878/

Comment 6

[Drew Willcoxon :adw]

The problem is that bug 1395387 changed when we call action.onIframeShown(), unintentionally I think.  Before that bug, we called it right after we created the activated-action panel and opened it: https://hg.mozilla.org/mozilla-central/annotate/1f21099e6fb5/browser/base/content/browser-pageActions.js#l247

Now, we're calling it in a popupshown listener: https://hg.mozilla.org/mozilla-central/annotate/45715ece25fc/browser/base/content/browser-pageActions.js#l257

The result is that the panel now appears before the Pocket code (in its bootstrap.js) can resize it.  So first it briefly appears smaller and square, and then it's resized.

This patch changes the popupshown listener to a popupshowing listener.  I think that's better than calling action.onIframeShown() directly in the method that simply creates the activated-action panel.  I renamed onIframeShown to onIframeShowing to better capture the meaning.  Pocket is the only non-test client in the tree.

The patch also reorganizes these listeners a bit so that they're organized by use case.  i.e., there's a popuphidden listener for removing the panel from the main popupset, a group of listeners for handling the case when there's an iframe, and a listener for handling the case when there's a subview.  That means that we add the same kind of listener more than once sometimes, but t'aint a problem IMO.

Comment 7

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a799b2b6e29

Comment 8

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcb45815614c

Comment 9

[:Gijs (he/him)]

Comment on attachment 8928640 [details]
Bug 1417036 - "Save to Pocket" drop down menu glitches.

https://reviewboard.mozilla.org/r/199878/#review205398

Nice catch, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e55d003816e0
"Save to Pocket" drop down menu glitches. r=Gijs

Comment 11

[Andrei Ciure[:aciure]]

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

Comment 12

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 59.0a1 (BuildId:20171117100127) on Windows 10 64bit, macOS 10.13 and Ubuntu 16.04 64bit.

Comment 13

[Drew Willcoxon :adw]

Attachment: Beta 58 patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1395387
[User impact if declined]: The Pocket panel animation isn't smooth, looks bad, and it's a regression from 57
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No, Emil already manually tested this
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Small change, affects only Pocket, code is covered by automated tests
[String changes made/needed]: None

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8929546 [details] [diff] [review]
Beta 58 patch

Fix a UI issue and was verified. Beta58+.

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/1e803860c220

Comment 16

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 58.0b8 (BuildId:20171130160223) on Windows 10 64bit, macOS 10.11 and Ubuntu 16.04 64bit.