1142550 - [Gallery][Screenshot] Taking a screenshot does not capture all of the UI on the screen

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Developer Tools, defect)

Description

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Description:
IF the user takes a screenshot anywhere on the phone, not all the UI will appear within the screenshot. For example if a screenshot is taken of the homescreen, apps will not show up in the screenshot. It will appear to be a screenshot of a blank homescreen with just the status bar and background.


Repro Steps:
1) Update a Flame to 20150312010235
2) Press and hold the power and volume down  buttons at the same time
3) Open the image via gallery or the notifications tray


Actual:
The screenshot will be missing most UI elements


Expected:
The screenshot will show exactly what was on the screen when the image was taken

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150312010235
Gaia: 0c4e8b0b330757e261b031b7e7f326ef419c9808
Gecko: 5334d2bead3e
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Repro frequency: 15/15
See attached: Logcat, Video - https://youtu.be/r11ACkcR2tU

Comment 1

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This issue does NOT occur on Flame 2.2

Screenshots will display correctly with all UI elements

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150312002501
Gaia: 572d60e0a440ee4af50bc6b6adad8876eadbdb4d
Gecko: 244e6ba3c20e
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 2

[No-Jun Park [:njpark]]

eeejay, with the new build with the fix for Bug 1142100, it now only takes the screenshot of the background image (See 1142550).  We're currently trying to find the regression range, but do you have an idea what would be the cause here?  thanks!

Comment 3

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Functional regression of a core feature that fails smoke tests.

Requesting a window.

Comment 4

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Attachment: Screenshot Logcat

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20150311074430
Gaia: 3325c49b8ca9b238399acf045efbbb7993269d66
Gecko: 0471951bd1b6
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20150311081833
Gaia: 3325c49b8ca9b238399acf045efbbb7993269d66
Gecko: ebd64d539f30
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Gaia is the same so it's Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=0471951bd1b6&tochange=ebd64d539f30

Possibly caused by Bug 1136784.

Comment 6

[No-Jun Park [:njpark]]

Hi lissyx, the bisection shows that above gecko checkin caused this smoketest failure today. Could you please take a look and see whether we can have the backout or fix before the next nightly build? Thanks!

Comment 7

[:gerard-majax]

I won't be able to work on this before tomorrow.

Comment 8

[:gerard-majax]

The problem probably comes from https://dxr.mozilla.org/mozilla-central/source/b2g/components/Screenshot.jsm#17 but I have no idea how to properly fix it ..

Comment 9

[:gerard-majax]

It means the document and/or the window we are getting from this is not the same as |document| and |window| we get from https://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#832

Comment 10

[Alexandre Poirot [:ochameau]]

(In reply to Alexandre LISSY :gerard-majax from comment #9)
> It means the document and/or the window we are getting from this is not the
> same as |document| and |window| we get from
> https://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.
> js#832

Before your patch you were using shell.html one and now the system app one.
From Screenshot.jsm you could fetch shell.html via:
   let shellWindow = Services.wm.getMostRecentWindow("navigator:browser");
Unfortunately, that wouldn't work on mulet, actually, it would "work", but it is going to take the whole browser as screenshot.
One alternative could be to fetch the system app iframe's document (i.e. shell.html). It is hacky but compatible with mulet :/

https://dxr.mozilla.org/mozilla-central/source/b2g/components/Screenshot.jsm#17
  let systemAppFrame = SystemAppProxy.getFrame();
  let window = systemAppFrame.ownerDocument.defaultView;

Comment 11

[:gerard-majax]

(In reply to Alexandre Poirot [:ochameau] from comment #10)
> (In reply to Alexandre LISSY :gerard-majax from comment #9)
> > It means the document and/or the window we are getting from this is not the
> > same as |document| and |window| we get from
> > https://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.
> > js#832

[...]

> 
> https://dxr.mozilla.org/mozilla-central/source/b2g/components/Screenshot.
> jsm#17
>   let systemAppFrame = SystemAppProxy.getFrame();
>   let window = systemAppFrame.ownerDocument.defaultView;

That seems to be working.

Comment 12

[:gerard-majax]

Attachment: Fix screenshot r=gwagner

Comment 13

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #12)
> Created attachment 8576839 [details] [diff] [review]
> Fix screenshot r=gwagner

This is the fix suggested by :ochameau. As far as I could test, it fixes the issue. I'm very sorry for this bustage, I tested a lot of things and it looks like I messed up :(.

Would you mind check if it fixes on your side too ?

Comment 14

[No-Jun Park [:njpark]]

Fix looks good, it takes pictures properly for landscape/portrait, various overlays, camera previews, and all app windows that I checked.  Thanks for the prompt fix! We should push the fix to master.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1d8b0c948fd3

Any way to write a test for this?

Comment 16

[Peter Bylenga [:PBylenga]]

Adding qawanted to verify this on next available nightly.

Comment 17

[No-Jun Park [:njpark]]

I just verified it on the nightly, and it looks good.

Comment 18

[Johan Lorenzo [:jlorenzo]]

Moving the bug to the component where the regression came from.