Closed Bug 1417036 Opened 7 years ago Closed 7 years ago

"Save to Pocket" drop down menu glitches

Categories

(Firefox :: Pocket, defect)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- verified
firefox59 --- verified

People

(Reporter: emilghitta, Assigned: adw)

References

Details

(Keywords: regression)

Attachments

(4 files)

Attached image Pocket.gif (deleted) —
[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.
Hi Drew! Can you please have a look into this ? Thanks!
Flags: needinfo?(adw)
What exactly is the glitch?
Flags: needinfo?(adw) → needinfo?(emil.ghitta)
Attached image Good.gif (deleted) —
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).
Flags: needinfo?(emil.ghitta)
I see, thanks for explaining further. I can reproduce it on the current Nightly on macOS, too.
Assignee: nobody → adw
Status: NEW → ASSIGNED
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 on attachment 8928640 [details] Bug 1417036 - "Save to Pocket" drop down menu glitches. https://reviewboard.mozilla.org/r/199878/#review205398 Nice catch, thanks!
Attachment #8928640 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e55d003816e0 "Save to Pocket" drop down menu glitches. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
This issue is verified fixed using Firefox 59.0a1 (BuildId:20171117100127) on Windows 10 64bit, macOS 10.13 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Blocks: 1395387
Attached patch Beta 58 patch (deleted) — Splinter Review
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
Attachment #8929546 - Flags: approval-mozilla-beta?
Comment on attachment 8929546 [details] [diff] [review] Beta 58 patch Fix a UI issue and was verified. Beta58+.
Attachment #8929546 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This issue is verified fixed using Firefox 58.0b8 (BuildId:20171130160223) on Windows 10 64bit, macOS 10.11 and Ubuntu 16.04 64bit.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: