Derek Harris [:DerekH] - Dont need info. Last day 6/5/15 Reporter
	Description • 9 years ago

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

	Derek Harris [:DerekH] - Dont need info. Last day 6/5/15 Reporter
	Comment 1 • 9 years ago

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

	No-Jun Park [:njpark]
	Comment 2 • 9 years ago

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!

	Peter Bylenga [:PBylenga]
	Comment 3 • 9 years ago

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

Requesting a window.

	Pi Wei Cheng [:piwei] (inactive)
	Comment 5 • 9 years ago

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.

	No-Jun Park [:njpark]
	Comment 6 • 9 years ago

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!

	:gerard-majax Assignee
	Comment 7 • 9 years ago

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

	:gerard-majax Assignee
	Comment 8 • 9 years ago

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 ..

	:gerard-majax Assignee
	Comment 9 • 9 years ago

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

	Alexandre Poirot [:ochameau]
	Comment 10 • 9 years ago

(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;

	:gerard-majax Assignee
	Comment 11 • 9 years ago

(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.

	:gerard-majax Assignee
	Comment 13 • 9 years ago

(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 ?

	No-Jun Park [:njpark]
	Comment 14 • 9 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 9 years ago

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

Any way to write a test for this?

	Peter Bylenga [:PBylenga]
	Comment 16 • 9 years ago

Adding qawanted to verify this on next available nightly.

	No-Jun Park [:njpark]
	Comment 17 • 9 years ago

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

	Johan Lorenzo [:jlorenzo]
	Comment 18 • 9 years ago

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