photon animated refresh button gets confused and gets stuck on disabled stop button

VERIFIED FIXED in Firefox 57

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: bkelly, Assigned: jaws)

Tracking

unspecified
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56+ disabled, firefox57 fixed)

Details

(Whiteboard: [reserve-photon-animation])

Attachments

(2 attachments)

STR:

1. Open a new nightly
2. Load this page in a new tab: https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
3. If you get a first time overlay, dismiss and go back to (2)

About 50% of the time I end up on a disabled stop button on this page.  I noticed because I wanted to reload, but the button wasn't available.

See attached screenshot.

I'm on windows 10 using 56.0a1 (2017-08-01) (64-bit).
Justin, who is the right person to ping for refresh/stop button animation changes?  I know you have blogged a bit about it in your photon newsletter.  Thanks.
Flags: needinfo?(dolske)

Comment 2

2 years ago
(In reply to Ben Kelly [:bkelly] from comment #1)
> Justin, who is the right person to ping for refresh/stop button animation
> changes?  I know you have blogged a bit about it in your photon newsletter. 
> Thanks.

Jared. :-)
Flags: needinfo?(dolske) → needinfo?(jaws)
Whiteboard: [photon-animation][triage]
Flags: qe-verify+
Priority: -- → P3
QA Contact: jwilliams
Whiteboard: [photon-animation][triage] → [reserve-photon-animation]
Using mozregression I tracked it down to this pushlog,
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0faada5c2f308f101ab7c54a87b3dce80b97d0e3&tochange=417eea89f792059e88be5e217e6f1a5cecf2d3d0

Of those, https://bugzilla.mozilla.org/show_bug.cgi?id=1376892 looks like the cause.

Perry, can you look in to this?
Flags: needinfo?(jaws) → needinfo?(jiangperry)
[Tracking Requested - why for this release]: Some webpages may cause the stop/reload button to stop working.
Assignee: nobody → jiangperry
Status: NEW → ASSIGNED
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)
> Using mozregression I tracked it down to this pushlog,
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=0faada5c2f308f101ab7c54a87b3dce80b97d0e3&tochange=417e
> ea89f792059e88be5e217e6f1a5cecf2d3d0
> 
> Of those, https://bugzilla.mozilla.org/show_bug.cgi?id=1376892 looks like
> the cause.
> 
> Perry, can you look in to this?

Will do.
Flags: needinfo?(jiangperry)
Actually, I think it might be bug 1375599 which caused this.
Blocks: 1375599
Flags: needinfo?(jjong)
Priority: P3 → P1
I have confirmed via local backout that bug 1375599 did not cause this. Also, adding `return true;` at the top of CombinedStopReload._shouldSwitch fixes the bug for me.
No longer blocks: 1375599
Flags: needinfo?(jjong)
I'm not able to reproduce this on Windows 10 64-bit with Nightly 56.0a1 (2017-08-01). To clarify, this can be seen by opening a fresh Nightly and going to https://beta.doodle.com/poll/w8zhuavpk6ya4kwr a few times when there's no Doodle loading screen?
Flags: needinfo?(bkelly)
(In reply to :Perry Jiang from comment #8)
> I'm not able to reproduce this on Windows 10 64-bit with Nightly 56.0a1
> (2017-08-01). To clarify, this can be seen by opening a fresh Nightly and
> going to https://beta.doodle.com/poll/w8zhuavpk6ya4kwr a few times when
> there's no Doodle loading screen?

Yes, I can reproduce it on a clean profile with a local build. Make sure that you are using the Beta and don't get moved back to the non-beta version.
At the beginning of _shouldSwitch I put the following log statement:
> console.log(`_shouldSwitch with ${aRequest && aRequest.originalURI && aRequest.originalURI.spec}`);

This gave me the following output:
// Open a new tab and load the page
> _shouldSwitch with https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with https://acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html
> _shouldSwitch with https://acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html
> _shouldSwitch with about:srcdoc
> _shouldSwitch with about:srcdoc
> _shouldSwitch with wyciwyg://0/https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with wyciwyg://1/https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with about:blank
> _shouldSwitch with about:blank
> _shouldSwitch with wyciwyg://6/https://x.vindicosuite.com/serve/?v=5;m=3;l=529015;c=1004737;b=4448846;ts=1501635174;u=%3Cpage_url_escaped%3E;r=%3Creferrer_escaped%3E;ad=CPekIBCqlA0Y5gsgASgEMNWIATjjvwFAta8TSLe2F1DBqT1YzsSPAmC_twlomvckcAF4A4gBAJABAJgBAKIBEzM5ODU4NDUzMDc2OTE2NTUyMjiyAQhDUkVBVElWRbgBAcABAMgBANABANgBAOgBxcC5g9or;xid=3985845307691655228;ep=1;pasmc=https%3A%2F%2Fnym1-ib.adnxs.com%2Fclick%3FpHA9Ctej0D8jZ2FPO_zNPwAAAIDCdSNAI2dhTzv8zT-kcD0K16PQP97fQ-Eqsdk92eX5oaA4hBtmIoFZAAAAAF1nYwBBDgAAvAQAAGUAAAATWKQDyxYOAAAAAABVU0QAVVNEAKAAWAKmUgAAAAABAQUCAQAAAK4A_iQL-AAAAAA.%2Fbn%3D7048%2Freferrer%3Dhttps%253A%252F%252Fbeta.doodle.com%252Fpoll%252Fw8zhuavpk6ya4kwr%2Fclickenc%3Dhttps%253A%252F%252Fgoogleads.g.doubleclick.net%252Fdbm%252Fclk%253Fsa%253DL%2526ai%253DCgwFtZCKBWYGDJsaxMavapPAE5-qqhUuF6Nbr6ASuusTcpwQQASDml9YlYMmWx4mQpOgPyAEJqAMBqgSrAU_QOPPH0gUI70OKiYUWkbE-rPfmIbi6vLQ9yR65oKTgLHg_mYlDvtn0PU57CezuXJtX3MW2uwrI2CbA1yCX5Gb25M04Lgqhyh1gn6FLG2cwmAU3BrM268oRb-7lHCKxx1_o_R1XWb9JlpNpY4JWB3yo3CzL01QTAdZp9JKJlBgNF5l2uBB3FPhs1lY9BuXJt8Fw2BYsaDs3NBwrLxTitpNOeGWDpDVW_vy91OAEA5AGAaAGTYAH3cSVZKgHr8obqAfdyhuoB6a-G9gHANIIBwiAQBABGADyCA5iaWRkZXItMTg1NzY4NsgT1pL8AdATANgTAw%2526num%253D1%2526pr%253Dappnexuswinpricemacro%2526cid%253DCAASEuRoxX7oczOXTMimTISkV3_ZLg%2526sig%253DAOD64_1ki0WbPBz8nTk3SxD8uxs8d-nzHA%2526client%253Dca-pub-3076890012741467%2526dbm_c%253DAKAmf-DE-80GQEQN_sqSuKwhek-6n0Lop_er5r1ZdUWuc7sWHaOwnLSskiWL_F7OtXSBv9ynuOK2%2526dbm_d%253DAKAmf-C3GwcF5owmWBJvW9Aqea5rgIjd2HKG2H01wXPFrfUpgag11TFoNQ35g7eE0IAhaQkSdwGKWSVh4-pEz14v5l_NE0jI55mu2GN1Na4LumkoQicNbtgysT3H6GAKdBJn0ZOjn8lhz6nPGy3yohI5g3I0I6hQxCkpKyaa2KmU3b93jZAmm6n7IBajN73DZ0_-qdmftZ0N%2526adurl%253D
> _shouldSwitch with about:blank
> _shouldSwitch with wyciwyg://7/https://x.vindicosuite.com/serve/?v=5;m=3;l=529013;c=1004737;b=4448848;ts=1501635174;u=%3Cpage_url_escaped%3E;r=%3Creferrer_escaped%3E;ad=CPWkIBCqlA0Y5gsgASgEMNWIATjjvwFAta8TSLe2F1DBqT1Y0MSPAmDBtwlojfckcAF4A4gBAJABAJgBAKIBEzYwMDg2NDc0MzQ2OTA2MDA4NjWyAQhDUkVBVElWRbgBAcABAMgBANABANgBAOgB48C5g9or;xid=6008647434690600865;ep=1;pasmc=https%3A%2F%2Fnym1-ib.adnxs.com%2Fclick%3FpHA9Ctej0D8jZ2FPO_zNPwAAAIDC9RtAI2dhTzv8zT-kcD0K16PQPyEKo4TzroN72eX5oaA4hBtmIoFZAAAAAIZAYgBBDgAAvAQAAGUAAAAVWKQDyxYOAAAAAABVU0QAVVNEANgCWgD2SQAAAAABAQUCAQAAAK4AGSUe6QAAAAA.%2Fbn%3D7048%2Freferrer%3Dhttps%253A%252F%252Fbeta.doodle.com%252Fpoll%252Fw8zhuavpk6ya4kwr%2Fclickenc%3Dhttps%253A%252F%252Fgoogleads.g.doubleclick.net%252Fdbm%252Fclk%253Fsa%253DL%2526ai%253DCT3yPZCKBWbX5JYmL3QGw_IDID-fqqoVLrejW6-gErrrE3KcEEAEg5pfWJWDJlseJkKToD8gBCagDAaoEqwFP0PUybuceYHAJWZ7fnBFEWSB4FR74PyVrp93a44byP9-PRGM13H4C4kNmxbOFfctGO-yvc--qSuDKHdwe1ETknrhCWmaM42XWckIKFIganASANjRftltDrfps7QGqBLzrnlHpi3JlX9pFpWqfDG31BjnOLPLkTrZbT1jNpkiLprklbZ7LvkSHn9ZnB8bKtW-cTvfMLrMiEEY-huNEAeTO3McGezz30_a4yJfgBAOQBgGgBk2AB93ElWSoB6_KG6gH3cobqAemvhvYBwDSCAcIgEAQARgA8ggOYmlkZGVyLTE4NTc2ODbIE9aS_AHQEwDYEwM%2526num%253D1%2526pr%253Dappnexuswinpricemacro%2526cid%253DCAASEuRohEqUCncku-CkscnmFITkgA%2526sig%253DAOD64_1HKqPYgzfOsvRxsXe2LixcxAekkw%2526client%253Dca-pub-3076890012741467%2526dbm_c%253DAKAmf-DLIpUgPagMr-gkiGFVy0OHq5wLXZG4BPUL60fQx6emlBFYTqAQdvSSS_6KklV3YMbWhrqI%2526dbm_d%253DAKAmf-ALhTJIWXGWTBzd2xC-6qn3xDyS6bZkukqBzWrw5eBom3u6MyBOeKKLmRhucgtX43QZ8bSl0L863CJrJMB6i1niwaXJ8Uy8kelyIcYHTPRb03k9rtvjFSdIKUKaOEz92eePYqd7YXYyuqS0wrr_NeRhzzT5gwzJv1GOGNt-U2Q4Nj18zF0Lb5BSQxIMgcNRu4MUBgbl%2526adurl%253D
> 
> // Close the tab, open a new tab and load the page again
> 
> _shouldSwitch with https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with https://acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html
> _shouldSwitch with https://acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html
> _shouldSwitch with about:srcdoc
> _shouldSwitch with about:srcdoc
> _shouldSwitch with wyciwyg://0/https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with wyciwyg://1/https://beta.doodle.com/poll/w8zhuavpk6ya4kwr
> _shouldSwitch with about:blank
> _shouldSwitch with about:blank

The interesting things I see here are wyciwyg scheme but we don't do anything special with that in _shouldSwitch. Also there is about:srcdoc and about:blank.
The other weird thing about this bug is that the reload button looks disabled but after bug 1376893 we shouldn't show it as disabled (greyed out) anymore.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> The other weird thing about this bug is that the reload button looks
> disabled but after bug 1376893 we shouldn't show it as disabled (greyed out)
> anymore.

Okay, this actually isn't so weird. Bug 1376893 only added new rules for #reload-button, but we're actually seeing a disabled #stop-button, so bug 1376893 doesn't apply here.
Track 56+ as new regression and might cause breakage for some webpages.
Flags: needinfo?(bkelly)
So could it be solved by only skipping the animation if it's for a top-level about: load?

The webProgress object (that is not passed as a parameter to _shouldSwitch, but could be passed very easily) has an .isTopLevel property that can be used to check this.
(In reply to :Felipe Gomes (needinfo me!) from comment #14)
> So could it be solved by only skipping the animation if it's for a top-level
> about: load?
> 
> The webProgress object (that is not passed as a parameter to _shouldSwitch,
> but could be passed very easily) has an .isTopLevel property that can be
> used to check this.

Yeah, that fixes it. Attaching a patch now...
Assignee: jiangperry → jaws

Comment 17

2 years ago
mozreview-review
Comment on attachment 8893125 [details]
Bug 1386255 - Switch the stop/reload button if the about: URL loaded is not top-level.

https://reviewboard.mozilla.org/r/164122/#review169472
Attachment #8893125 - Flags: review?(felipc) → review+

Comment 18

2 years ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d9420078ea8c
Switch the stop/reload button if the about: URL loaded is not top-level. r=Felipe
Iteration: --- → 57.1 - Aug 15
https://hg.mozilla.org/mozilla-central/rev/d9420078ea8c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
QA Contact: jwilliams → stefan.georgiev
User Agent    Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build   ID    20170830100230

This bug fix is Verified with latest Nightly 57.0a1.
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: FIXED → ---
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Flags: qe-verify+

Updated

2 years ago
Depends on: 1402493
You need to log in before you can comment on or make changes to this bug.