1377128 - Screenshots overlay button icons are not displayed on latest Nightly build

Status: VERIFIED FIXED

(Firefox :: Screenshots, defect)

Description

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Attachment: Screenshots.png

[Affected versions]:
- Nightly 56.0a1, Build ID:20170629030206

[Affected Platforms]:
- All Windows
- All Mac
- All Linux

[Steps to reproduce]:
1. Start the latest Nightly build and click the "Screenshots" icon from browser toolbar.
2. Skip the "Tour" pop-up.
3. Observe the "Save full page", "Save visible" and "My Shots" buttons
4. Perform a selection and observe the "Cancel" and "Download" buttons.

[Expected result]:
Step 3 & Step4: The button icons are correctly displayed.

[Actual results]:
Step 3 & Step4: The button icons are no longer displayed.

[Regression]:
Last good revision: 6cc6282c67c75b86289d944a31cb39f5a5639925
First bad revision: 3358bef7a047d347e576bbb83d1461b3776fc5fb
Pushlog: https://goo.gl/z1u9Kj

Hiroyuki, can you please take a look at this issue? Is there a chance that the fix for bug 1375787 affected this?

[Notes]:
- Attached a screenshot with the issue.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

(In reply to Cosmin Muntean [:CosminMCG], Desktop Engineering QA from comment #0)
> [Regression]:
> Last good revision: 6cc6282c67c75b86289d944a31cb39f5a5639925
> First bad revision: 3358bef7a047d347e576bbb83d1461b3776fc5fb
> Pushlog: https://goo.gl/z1u9Kj
> 
> Hiroyuki, can you please take a look at this issue? Is there a chance that
> the fix for bug 1375787 affected this?

Definitely not.  Bug 1375787 is only for stylo.

Comment 2

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have tried again to perform a regression range, but I have obtained the same results. So if bug 1375787 did not affect this issue, then probably bug 1334550?

Haik, can you also please look at this? Is there any way that the fix for bug 1334550 can affect this?

Comment 3

[Haik Aftandilian [:haik]]

It looks like it was caused by the fix for bug 1334550. As a workaround, you can set extensions.webextensions.protocol.remote=false and restart the browser.

Comment 4

[Haik Aftandilian [:haik]]

Still debugging, but the problem appears to be that the content type isn't being set by the moz-extension remoting code. For SVG images, which are used for the Screenshots overlay button icons, the images fail to load in the content process. I haven't noticed any problems with other file types such as js or jpegs.

SVG images don't work because in ImageFactory::CreateImage() we have the following code.

  // Select the type of image to create based on MIME type.
  if (aMimeType.EqualsLiteral(IMAGE_SVG_XML)) {
    return CreateVectorImage(aRequest, aProgressTracker, aMimeType,
                             aURI, imageFlags, aInnerWindowId);
  } else {
    return CreateRasterImage(aRequest, aProgressTracker, aMimeType,
                             aURI, imageFlags, aInnerWindowId);
  }

The content type for the remote extension image ends up being "application/x-unknown-content-type" which puts us in CreateRasterImage() instead of CreateVectorImage().

In the remoting code, explicitly setting the content type on the SimpleChannel for files ending in .svg to "image/svg+xml" works around the problem, but we should be able to do this in a general way that works for all types.

Comment 6

[Haik Aftandilian [:haik]]

In the child process, we need to set the content type for the placeholder SimpleChannel used in remoting. For local moz-extension loads (not remoted) the content type is set as a result of calling nsFileChannel::OpenContentStream() and the file extension is used to get a content type which is passed to nsBaseChannel::SetContentType(). We can accomplish the same thing using the file extension from the URI.

Comment 7

[Haik Aftandilian [:haik]]

Attachment: Bug 1377128 - Screenshots overlay button icons are not displayed on latest Nightly build.

Set the content type on remote channels in the moz-extension protocol handler.

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

Comment 8

[Haik Aftandilian [:haik]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=310921294233baf86f329365923abca5ba812bc4
https://treeherder.mozilla.org/#/jobs?repo=try&revision=48560b95a73f6536cd2bcfe2eff49a18d6f0dab6

Comment 9

[Haik Aftandilian [:haik]]

Comment on attachment 8883468 [details]
Bug 1377128 - Screenshots overlay button icons are not displayed on latest Nightly build.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154372/diff/1-2/

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 8883468 [details]
Bug 1377128 - Screenshots overlay button icons are not displayed on latest Nightly build.

https://reviewboard.mozilla.org/r/154372/#review159678

Comment 11

[Haik Aftandilian [:haik]]

Comment on attachment 8883468 [details]
Bug 1377128 - Screenshots overlay button icons are not displayed on latest Nightly build.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154372/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4179ee159b3
Screenshots overlay button icons are not displayed on latest Nightly build. r=jimm

Comment 13

[Carsten Book [:Tomcat]]

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

Comment 14

[Saheda Reza [:Antora]]

I have reproduced this bug according to (2017-06-29)

Fixing bug is verified on Latest Nightly & Latest Beta,
Latest Nightly--
Build ID    :20171027220059
User Agent  :Mozilla/5.0 (Windows NT 6.1; rv:58.0) Gecko/20100101 Firefox/58.0

Latest Beta--
Build ID    :20171026211016
User Agent  :Mozilla/5.0 (Windows NT 6.1; rv:57.0) Gecko/20100101 Firefox/57.0

Tested OS-Windows7 32bit

Comment 15

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I'm cleaning up the old issues. The Screenshots feature has improved a lot since the issue was fixed. However, based on comment 14 this issue was verified, and considering this I will mark this bug as verified fixed.