Paul [pwd] Reporter
	Description • 9 years ago

The ability to reload an image without reloading the whole page is missing from the contextual menu with e10s enabled.

	Tracy Walker [:tracy]
	Comment 1 • 9 years ago

Paul, can you reproduce this with a fresh profile (or with all your addons disabled)?

	Paul [pwd] Reporter
	Comment 2 • 9 years ago

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #1)
> Paul, can you reproduce this with a fresh profile (or with all your addons
> disabled)?

With all add-ons disabled I can reproduce it. I haven't tried with a fresh profile.

	Mike Conley (:mconley) (:⚙️)
	Comment 3 • 9 years ago

Hey Ian - feel like taking a crack at this one?

	Ian Moody [:Kwan] (UTC+1)
	Comment 4 • 9 years ago

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> Hey Ian - feel like taking a crack at this one?
I can take a look.

Paul could you show me an example of where you are seeing this behaviour please?  The testing page I used for bug 1141160 is still causing "Reload Image" to be shown in e10s.
http://kwan.perix.co.uk/mozilla/cpow/cpowReloadImageTest.html

	Paul [pwd] Reporter
	Comment 5 • 9 years ago

It works on the page you linked to, but Tracy and my avatar's on this page aren't displaying the contextual menu item. Could it be to do with hyperlinked images? Which would fit with where I'm usually seeing images on my travels.

	Ian Moody [:Kwan] (UTC+1)
	Comment 6 • 9 years ago

(In reply to Paul [pwd] from comment #5)
> It works on the page you linked to, but Tracy and my avatar's on this page
> aren't displaying the contextual menu item. Could it be to do with
> hyperlinked images? Which would fit with where I'm usually seeing images on
> my travels.

You mean you are seeing "Reload Image" on them in non-e10s?  I don't believe that should be happening, since the item shouldn't be there for images that have loaded correctly.  I'm certainly not seeing it on those avatars in release or in non-e10s nightly.

	Paul [pwd] Reporter
	Comment 7 • 9 years ago

Sorry, Tracy's avatar hasn't appeared here on bugzilla since filing this bug and then as Sod's law would have it, after my last response, both his and mine came back up again. Do you have a test case where the image is wrapped in a hyperlink tag, as I've been looking for another test case and images seem to be loading everywhere perfectly right now.

	Ian Moody [:Kwan] (UTC+1)
	Comment 8 • 9 years ago

I've updated the linked test to include one wrapped in a link, and I'm still seeing the menuitem.  It might have been dependent on exactly why the images weren't loading for you, which'll be harder to debug.

	Paul [pwd] Reporter
	Comment 9 • 9 years ago

The new test case worked also, luckily I thought I'd give it a try tracking down one which showed the issue and got lucky. The video is too large to attach here, but here's the dropbox. Excuse the fact it was done from my phone: https://www.dropbox.com/s/fchq9hn16j8wiqp/20150508_104031.mp4?dl=0

	Ian Moody [:Kwan] (UTC+1)
	Comment 10 • 9 years ago

And you definitely did see the menu item in non-e10s?

Do you know why the image didn't load the first time?  This isn't really possible to debug without recreating the network conditions that cause stuff not to load for you.  If I make the domain the image is on resolve to ::1 using the hosts file to make the image not load, I still don't get the "Reload Image" option in release.

	Paul [pwd] Reporter
	Comment 11 • 9 years ago

(In reply to Ian Moody [:Kwan] from comment #10)
> And you definitely did see the menu item in non-e10s?
100%

> Do you know why the image didn't load the first time?  This isn't really
> possible to debug without recreating the network conditions that cause stuff
> not to load for you.  If I make the domain the image is on resolve to ::1
> using the hosts file to make the image not load, I still don't get the
> "Reload Image" option in release.

I've been checking this out over the past couple of days. It seems that the image just doesn't get requested properly. For example I'll open a page and I'll see the broken image thing pretty much straight away. I'll also see the same issue with tabs that are being restored where the image doesn't load.

	Ian Moody [:Kwan] (UTC+1)
	Comment 12 • 9 years ago

(In reply to Paul [pwd] from comment #11)
> (In reply to Ian Moody [:Kwan] from comment #10)
> > And you definitely did see the menu item in non-e10s?
> 100%
> 
> > Do you know why the image didn't load the first time?  This isn't really
> > possible to debug without recreating the network conditions that cause stuff
> > not to load for you.  If I make the domain the image is on resolve to ::1
> > using the hosts file to make the image not load, I still don't get the
> > "Reload Image" option in release.
> 
> I've been checking this out over the past couple of days. It seems that the
> image just doesn't get requested properly. For example I'll open a page and
> I'll see the broken image thing pretty much straight away. I'll also see the
> same issue with tabs that are being restored where the image doesn't load.

Hmm.  This is probably getting a bit beyond me, but I have one last possibly-helpful debugging step I can suggest.
The next time this happens, open the Browser Toolbox https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox , use the inspector in there to inspect the offending image, and then in the console of the toolbox run the code:
let request = $0.getRequest(Components.interfaces.nsIImageLoadingContent); request.imageStatus

do that for both e10s and non-e10s and post the values back here (they should be simple integers)

	Paul [pwd] Reporter
	Comment 13 • 9 years ago

The Browser Toolbox Inspector wouldn't let me inspect page elements and the normal Inspector would allow it, but when I attempted to run the command in the console it would return "TypeError: $0.getRequest is not a function".

	Paul [pwd] Reporter
	Comment 14 • 9 years ago

Could this be linked to the way that e10s interacts with the cache?

	Ian Moody [:Kwan] (UTC+1)
	Comment 15 • 9 years ago

(In reply to Paul [pwd] from comment #13)
> The Browser Toolbox Inspector wouldn't let me inspect page elements and the
> normal Inspector would allow it, but when I attempted to run the command in
> the console it would return "TypeError: $0.getRequest is not a function".
Ah, right, of course, the Browser Toolbox can't reach content during e10s.  I forgot.  And the Browser Content Toolbox doesn't have an Inspector...

hmm, slightly more convoluted way, but seems to work, next time the problem happens:
- Open the Browser Content Toolbox (BCT)
- Go to the Debugger tab in the BCT
- find the file content.js (in chrome://browser section)
- set a breakpoint at the line "  let defaultPrevented = event.defaultPrevented;" (right after "let handleContentContextMenu = function (event) {"), should be around line 62 of the file.
- right-click on the offending image in the browser, and the Debugger in the BCT should get auto-focused
- switch to the console tab in BCT, run this code:
let request = event.target.getRequest(Components.interfaces.nsIImageLoadingContent); request.imageStatus

	:Felipe Gomes (needinfo for replies!)
	Comment 16 • 9 years ago
testcase

So I managed to build a testcase that works reliably to reproduce this. See URL. While the images are loading the context menu appear, but they don't show up after they are in a broken state.

With the testcase I verified that this is a not an e10s issue, as it happens both in e10s and non-e10s windows.

I then bisected using mozregression and found the following range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2114ef80f6ae&tochange=b62ccf3228ba

The most suspect bug in that range is bug 1084136, which reworked the image load status code.

	Sotaro Ikeda [:sotaro]
	Comment 17 • 9 years ago

(In reply to :Felipe Gomes (needinfo me!) from comment #16)
> 
> The most suspect bug in that range is bug 1084136, which reworked the image
> load status code.

:seth, can you take a look? You worked bug 1084136. Thanks.

	Sotaro Ikeda [:sotaro]
	Comment 18 • 8 years ago

:tn can you comment to the bug?

	arni2033
	Comment 19 • 8 years ago

My use case is fairly simple and more common. I press Escape key (to cancel loading of ads & heavy page itself) when page is still loading, once I see see that image is available (even if not loaded).

BTW, I can reproduce the bug on this page:   data:text/html,<img src="http://asdf.asdf" alt="zxcv">

	Masatoshi Kimura [:emk]
	Comment 20 • 8 years ago

The "Reload Image" menu item checks if imgIRequest::STATUS_LOAD_COMPLETE is set.[1][2]
After bug 1084136, imgIRequest::STATUS_LOAD_COMPLETE will be set even if the image has an error. I think this change is intentional because the aim of bug 1084136 is making the image state monotonic. So the frontend side should adapt the check.

[1] https://dxr.mozilla.org/mozilla-central/rev/1a5b53a831e5a6c20de1b081c774feb3ff76756c/browser/base/content/nsContextMenu.js#698-699
[2] https://dxr.mozilla.org/mozilla-central/rev/1a5b53a831e5a6c20de1b081c774feb3ff76756c/browser/base/content/nsContextMenu.js#278

	Masatoshi Kimura [:emk]
	Comment 21 • 8 years ago

Seth, please teach me if my guess is wrong.

	Florian Quèze [:florian]
	Comment 23 • 8 years ago
mozreview-review

Comment on attachment 8785780 [details]
Bug 1161599 - Show "Reload Image" if the image has an error.

https://reviewboard.mozilla.org/r/74852/#review73570

Looks good to me.

	Pulsebot
	Comment 24 • 8 years ago

Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/6233844593ae
Show "Reload Image" if the image has an error. r=florian

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6233844593ae

	Bogdan Maris, Desktop QA
	Comment 26 • 8 years ago

Reproduced the initial issue using old Nightly from 2016-08-30. Verified that Reload images appears in Context menu every time an image fails to load using Firefox 51 beta 10 across platforms (Windows 10 64bit, Mac OS X 10.12.1 and Ubuntu 16.04 64bit).