Refresh button gets stuck in disabled stop state when extension loads content in page

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: u602702, Assigned: jaws)

Tracking

({regression})

57 Branch
Firefox 58
regression
Points:
---
Bug Flags:
qe-verify ?

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 disabled, firefox57+ verified, firefox58+ verified)

Details

(Whiteboard: [reserve-photon-animation])

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170917031738

Steps to reproduce:

Install "Reddit Enhancment suite and open any image, it will make the refresh button unresponsive and unable to be clicked. If you don't load anything, the button works as intended, as does it without the plugin. 

This is a very popular plugin and this could be affecting quite a few people.


Actual results:

The refresh button stops working when an image is loaded, forcing use of f5 or ctrl-r.


Expected results:

The refresh button should be able to be clicked after the image has finished loading.

Comment 1

2 years ago
Regression range between 2017-08-01 and 2017-08-04, most likely regressed by Bug 1386255.
Blocks: 1386255
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox55: --- → unaffected
status-firefox56: --- → disabled
status-firefox57: --- → affected
status-firefox58: --- → affected
status-firefox-esr52: --- → unaffected
Component: Untriaged → Toolbars and Customization
Ever confirmed: true
Keywords: regression
Summary: Refresh button becomes unresponsive when loading images from certain websites, specifically Reddit with the RES plugin. → Refresh button gets stuck in disabled stop state when extension loads content in page
Whiteboard: [reserve-photon-animation]
Whiteboard: [reserve-photon-animation] → [photon-animation] [triage]
[Tracking Requested - why for this release]: add-on related issue that can cause the browser chrome to get in a broken state
Assignee: nobody → jaws
Status: NEW → ASSIGNED
tracking-firefox57: --- → ?
Priority: -- → P1
Whiteboard: [photon-animation] [triage] → [reserve-photon-animation]
tracking-firefox58: --- → +

Updated

2 years ago
See Also: → bug 1401943
Flags: qe-verify?
I have confirmed via mozregression that bug 1386255 is the cause of this.
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8912919 [details]
Bug 1402493 - Don't prevent switching back to Reload for top level about: loads.

https://reviewboard.mozilla.org/r/184230/#review189696

Tentative r+, see comment below.

::: browser/base/content/browser.js:5057
(Diff revision 1)
>      }
>      this.reload.setAttribute("displaystop", "true");
>    },
>  
>    switchToReload(aRequest, aWebProgress) {
> -    if (!this.ensureInitialized() || !this._shouldSwitch(aRequest, aWebProgress) ||
> +    if (!this.ensureInitialized() || !this.reload.hasAttribute("displaystop")) {

Your reasoning makes sense, but are you sure we should do this even if there's no request or the request has no URI at all? It doesn't look like we check that right now, and neither does the caller.
Attachment #8912919 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8912919 [details]
Bug 1402493 - Don't prevent switching back to Reload for top level about: loads.

https://reviewboard.mozilla.org/r/184230/#review189696

> Your reasoning makes sense, but are you sure we should do this even if there's no request or the request has no URI at all? It doesn't look like we check that right now, and neither does the caller.

The _shouldSwitch function returns true if there is no aRequest and it also returns true if there is no aRequest.originalURI, so this isn't a change in behavior for those two inputs.

Comment 7

2 years ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e595d08c3ee2
Don't prevent switching back to Reload for top level about: loads. r=Gijs

Comment 8

2 years ago
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> Comment on attachment 8912919 [details]
> Bug 1402493 - Don't prevent switching back to Reload for top level about:
> loads.
> 
> https://reviewboard.mozilla.org/r/184230/#review189696
> 
> > Your reasoning makes sense, but are you sure we should do this even if there's no request or the request has no URI at all? It doesn't look like we check that right now, and neither does the caller.
> 
> The _shouldSwitch function returns true if there is no aRequest and it also
> returns true if there is no aRequest.originalURI, so this isn't a change in
> behavior for those two inputs.

Ah, of course it does - clearly I misread it. Down with 6-line conditionals, I say! ;-)

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e595d08c3ee2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Hi Jared, should we uplift this to Beta57?
Flags: needinfo?(jaws)
tracking-firefox57: ? → +
Comment on attachment 8912919 [details]
Bug 1402493 - Don't prevent switching back to Reload for top level about: loads.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1386255
[User impact if declined]: stop/reload button may get stuck on stop
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: reverts to behavior that we had for the stop->reload switching pre-57.
[String changes made/needed]: none
Flags: needinfo?(jaws)
Attachment #8912919 - Flags: approval-mozilla-beta?
Comment on attachment 8912919 [details]
Bug 1402493 - Don't prevent switching back to Reload for top level about: loads.

Photon related, Beta57+
Attachment #8912919 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 13

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/0fe06c5dd395
status-firefox57: affected → fixed

Comment 14

2 years ago
This bug is verified on Firefox 57.0b5 (20171002181526) and Firefox 58.0a1 (20171002220204) under Windows 10 64bit/Mac OS X 10.12.3.

Please see the attached video.

Updated

2 years ago
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
status-firefox58: fixed → verified
You need to log in before you can comment on or make changes to this bug.