Unable to take screenshots

VERIFIED FIXED

Status

Firefox OS
Gaia::System
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: kgrandon, Assigned: dwi2)

Tracking

({regression, smoketest})

unspecified
x86
Mac OS X
regression, smoketest
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified)

Details

(Whiteboard: [ft:conndevices])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Possibly related to the recent hardware button dispatching changes, I'm now unable to take screenshots on the flame.

STR:
- Hold power + Volume Down buttons

Expected Result:
A screenshot is taken, and a notification appears.

Actual Result:
No screenshot is taken.

Gaia-Rev        c5bad6d78c5fe168e3bb894fc5cb70902c9b19b1
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/87d4d6d5facc
Build-ID        20141121160202
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  39
FW-Date         Thu Oct 16 18:19:14 CST 2014
Bootloader      L1TC00011880
(Reporter)

Comment 1

3 years ago
Tzu-Lin - Could you check to see if this is related to bug 1014418?
Flags: needinfo?(tzhuang)
(Assignee)

Comment 2

3 years ago
I am working on it.
(keep the ni flag for reminding)
(Assignee)

Updated

3 years ago
Assignee: nobody → tzhuang
(Assignee)

Comment 3

3 years ago
Hi Kevin,

I am able to take screenshot on flame with volume-down + power button, using today's build and latest gaia on master

Gaia-Rev        aad40f6d6eb8f626c6a20db55b9f00d2e832f113
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/8c02f3280d0c
Build-ID        20141123160201
Version         36.0a1

https://www.youtube.com/watch?v=mGA6xfcn-aM

But the time latency of taking screenshot is longer than it is before. Because we are listening to key events (like mozbrowserbeforekeydown/mozbrowserbeforekeyup, ...etc ) from gecko. Gecko needs more time to decide which frame to dispatch these events.

I am wondering what is your STR of unable to take screenshots?
Flags: needinfo?(kgrandon)
(Assignee)

Updated

3 years ago
Assignee: tzhuang → nobody
(Assignee)

Updated

3 years ago
Flags: needinfo?(tzhuang)
(Assignee)

Comment 4

3 years ago
If timing issue matters, we might need to ask UX's opinion. 

Maybe we have to shorten 'hold interval' of volume-down + power.
(Reporter)

Comment 5

3 years ago
(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #3)
> I am wondering what is your STR of unable to take screenshots?

Still able to reproduce on latest Flame KK nightly build. Both volume down and power buttons work individually, but not for taking a screenshot.

I don't think it's a timing issue as eventually when holding the two buttons down the phone will power off instead of taking a screenshot. Trying to debug this...
Flags: needinfo?(kgrandon)
(Assignee)

Comment 6

3 years ago
May I know your gecko and gaia version and maybe base image version? 

It looks weird to me.
(Reporter)

Comment 7

3 years ago
Running the check_versions.sh script I see the following details: 

Gaia-Rev        c5bad6d78c5fe168e3bb894fc5cb70902c9b19b1
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/8c02f3280d0c

The gecko build points to Nov 19, which is a bit weird because I *just* ran the nightly OTA update, so maybe the builds are stale? I'm also building gecko myself now to test.
Tony, do you think we should add this to our daily smoke tests? I can't file bugs right now because screenshots are not working for me.
Flags: needinfo?(tchung)

Comment 9

3 years ago
(In reply to Gregor Wagner [:gwagner] from comment #8)
> Tony, do you think we should add this to our daily smoke tests? I can't file
> bugs right now because screenshots are not working for me.
Flags: needinfo?(tchung)
Keywords: regressionwindow-wanted
I can repro the bug with this build:
Gaia-Rev        c5bad6d78c5fe168e3bb894fc5cb70902c9b19b1
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/8c02f3280d0c
Build-ID        20141124040203
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  40
FW-Date         Tue Oct 21 15:59:42 CST 2014
Bootloader      L1TC10011880

I am aware of the time change, but in this case the shot does not happen at all. my STR is:
- reboot,
- on homescreen, take screenshot by power+voldown 

the phone restarts instead of taking screenshots.
(In reply to Tony Chung [:tchung] from comment #9)
> (In reply to Gregor Wagner [:gwagner] from comment #8)
> > Tony, do you think we should add this to our daily smoke tests? I can't file
> > bugs right now because screenshots are not working for me.

Yes, i think thats a fair request to add for daily test coverage.   not able to screenshot breaks QA and dev testing which is needed for development cycle.  Peter, please make sure this is included in 2.2 testing.

Lets get a regression window please.
Keywords: qawanted
I'll add it in to Smoke Test coverage, for us we can take screenshots if we first depress volume down and then hit power.
bug 1031560 changed the hold to 750ms, I think?

I get the same result as PeterB.
QA Contact: pcheng
(Reporter)

Comment 14

3 years ago
Hmm, is the bug then that pressing the power button first no longer works? This feels like a regression to me, can we still get a window for this?
Confirmed both the bug and workaround method on comment 12. Holding down both Power button and volume down button at the same time will reboot the phone after a couple of seconds; if I press volume down button first then add power button, screenshot can be taken.

2.1 is unaffected. Getting the window now.
status-b2g-v2.1: --- → unaffected
status-b2g-v2.2: --- → affected
Keywords: qawanted
Added it to smoketests.  This is failing smoketests, 

https://moztrap.mozilla.org/manage/cases/?pagenumber=1&pagesize=100&sortfield=created_on&sortdirection=desc&filter-id=14645
blocking-b2g: --- → 2.2?
Keywords: qaurgent, smoketest
b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20141107024412
Gaia: 3ca3172b9122fedf46c8411e92c48b724104aae5
Gecko: 45b2fb3afbce
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20141107030312
Gaia: 940d52127168dc694882967ccae37f6e294c9566
Gecko: 60cc0b101ce9
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Gaia & Last Working Gecko - issue DOES repro
Gaia: 940d52127168dc694882967ccae37f6e294c9566
Gecko: 45b2fb3afbce

First Broken Gecko & Last Working Gaia - issue does NOT repro
Gaia: 3ca3172b9122fedf46c8411e92c48b724104aae5
Gecko: 60cc0b101ce9

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/3ca3172b9122fedf46c8411e92c48b724104aae5...940d52127168dc694882967ccae37f6e294c9566

Caused by Bug 1014418.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Keywords: qaurgent, regressionwindow-wanted
Broken by Bug 1014418 as suspected in comment 1.  Tzu-Lin what is your recommendation
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado) → needinfo?(tzhuang)
Blocks: 1014418
(Assignee)

Comment 19

3 years ago
Now I could reproduce with volume-down and power key pressed at the same time or power key pressed and held first then volume-down.

It could be caused by bug 1014418. I am looking into it.
Flags: needinfo?(tzhuang)
(Assignee)

Updated

3 years ago
Assignee: nobody → tzhuang
(Assignee)

Comment 20

3 years ago
I found the root cause.

TL; DR
System app starts listening to keydown/keyup event after bug 1014418. And after bug 989198, gecko dispatches one more keydown/keyup event to embedder(System app). This extra keydown/keyup event confuses state machine in hardware_buttons.js. We should ignore this extra keydown/keyup event in System app.


The actual key event sequence of the patch in bug 989198 would be:
  1. mozbrowserbeforekeydown event in embedder frame (i.e. System app)
  2. keydown event in embedder frame (*)
  3. keydown event in embedded frame
  4. mozbrowserafterkeydown event in embedder frame

(*): Step 2 is the reason why we cannot take screenshot. Because we process keydown event and translate to volume-down-button-press at step 2, and translate it again at step 4. 

However we must listen to keydown/keyup event in System app, because sometimes the focus is on System app (For example, when user pull down utility tray). I think the solution would be: when System app receives keydown/keyup, we examine its event.target. If it is for iframe, we should ignore this key event.
(Assignee)

Comment 21

3 years ago
Created attachment 8528131 [details] [review]
pull request

Hi Tim, 

Would you help to review the patch? thanks

The rationale of this patch is described at https://bugzilla.mozilla.org/show_bug.cgi?id=1103339#c20 and https://bugzilla.mozilla.org/show_bug.cgi?id=1094066#c1
Attachment #8528131 - Flags: review?(timdream)

Updated

3 years ago
See Also: → bug 1031560
Component: Gaia::System::Input Mgmt → Gaia::System
Whiteboard: [FT:Stream3]
Whiteboard: [FT:Stream3] → [ft:conndevices]
Attachment #8528131 - Flags: review?(timdream) → review+
(Assignee)

Comment 22

3 years ago
rebase to latest master and wait for Gaia-Try result
https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=e00d1d5d8158
(Assignee)

Comment 23

3 years ago
landed on master
https://github.com/mozilla-b2g/gaia/commit/7878734c7baf7d0c665c2f838168fce21cdc95b6
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
This issue is verified fixed on Flame 2.2.

Result: The screenshot is taken when the power button and volume down buttons are pressed at the same time.

Device: Flame 2.2 (319mb, KK, Full Flash)
BuildID: 20141201040205
Gaia: 39214fb22c203e8849aaa1c27b773eeb73212921
Gecko: 08be3008650f
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 37.0a1 
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
status-b2g-v2.2: affected → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
blocking-b2g: 2.2? → 2.2+
You need to log in before you can comment on or make changes to this bug.