Long press home button during an activity, app preview is shown black

VERIFIED FIXED in Firefox OS v2.0

Status

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

People

(Reporter: ankit93040, Assigned: aus)

Tracking

unspecified
2.1 S5 (26sep)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [g+][LibGLA,TD94569,QE1, B] [systemsfe][p=8][LibGLA,TD95653,WW, B])

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.143 Safari/537.36

Steps to reproduce:

Launch ringtone app
log press home button



Actual results:

See the app preview appears all black


Expected results:

app preview should be shown
(Reporter)

Updated

4 years ago
Whiteboard: [g+][LibGLA,TD94569,QE1, B]
(Reporter)

Comment 1

4 years ago
Created attachment 8483329 [details]
CAM00062.mp4
Flags: needinfo?(squibblyflabbetydoo)
(Reporter)

Comment 2

4 years ago
Either a gecko or ringtone issue (less probabilty).
(Reporter)

Updated

4 years ago
Component: General → Gaia::Ringtones
(Reporter)

Comment 3

4 years ago
Don't know whether to needinfo you or not, but I doubt even ringtone has something in these issue.
Flags: needinfo?(alive)
Put in Sam's radar.
Flags: needinfo?(alive) → needinfo?(sfoster)

Comment 5

4 years ago
This isn't a ringtones bug. It happens with any activity.
Component: Gaia::Ringtones → Gaia::System
Flags: needinfo?(squibblyflabbetydoo)
Summary: [Ringtone]Long press home button, app preview is shown black → Long press home button during an activity, app preview is shown black
What I'm seeing on master is that sometimes the screenshot takes 3-5sec to appear when you enter cards view. So, initially you see black, but if you wait the screenshot eventually shows up. This might be a recent regression as it seems to me the screenshots used to be available much faster - close to instantly. 

Can you confirm and provide STR? Because having no screenshot at all - for activities specifically - is quite a different bug to screenshots in general being very slow to appear.
Flags: needinfo?(squibblyflabbetydoo)
Flags: needinfo?(sfoster)
Flags: needinfo?(ankit93040)

Comment 7

4 years ago
I don't see a screenshot at all for activities. However, if I drag to scroll the app list and then let go, the screenshot suddenly appears.
Flags: needinfo?(squibblyflabbetydoo)
(Reporter)

Comment 8

4 years ago
Created attachment 8483937 [details]
CAM00063.mp4

Please see the video for STR.

Go to settings->Ringer/Alerts->Ringtone APP->(Press long on Home button)

See the cards view. Complete black.

However, this happens only once for ringtone app. I mean for producing these issue, everytime you have to do the STR. 

If you remain in Ringtone app & press long home button then you wont see this happening.

Thanks
Attachment #8483329 - Attachment is obsolete: true
Flags: needinfo?(ankit93040)
(Reporter)

Comment 9

4 years ago
I tested it on v2.0
[Blocking Requested - why for this release]:

[Blocking Requested - why for this release]:

Just reproduced this again with these STR: 

1. Browse to a website in system browser
2. Using overflow menu (...) select Add to Homescreen
3. 'Add link' bookmark editor activity window appears
4. Long press home button

Expected results: 
Cards view shows screenshot of the bookmark editor

Actual result: 
Cards view shows a black card titled with the web page title but no screenshot or icon. 

I can also confirm comment #7, dragging and releasing the card causes the screenshot to show up. Nom'ing for blocking 2.1. Do we also need to have this block 2.0?
Status: UNCONFIRMED → NEW
blocking-b2g: --- → 2.1?
Ever confirmed: true
Whiteboard: [g+][LibGLA,TD94569,QE1, B] → [g+][LibGLA,TD94569,QE1, B] [systemsfe]
(Reporter)

Comment 11

4 years ago
Hi Sam

Do we also need to have this block 2.0?
<<Ankit>> Please do it too..
not a regression but high priority bug for backlog
blocking-b2g: 2.1? → backlog
[Blocking Requested - why for this release]:

Suggest to nominate to 2.1 blocker since the black screen on the card view will leave bad user experience to end users, also the RST is reasonable. So suggest to nominate to 2.1. Also originally partner ask this for 2.0 blocker, so fixing this one in 2.1 give partner a chance to cheery-pick it to their code base

Thanks

Vance
blocking-b2g: backlog → 2.1?
(Reporter)

Updated

4 years ago
Whiteboard: [g+][LibGLA,TD94569,QE1, B] [systemsfe] → [g+][LibGLA,TD94569,QE1, B] [systemsfe][LibGLA,TD95653,WW, B]
Hi Sam -

Partner is asking for the fix of this issue on 2.0, do you think it is possible you also land the fix on 2.0 as well?

Thanks

Vance
Flags: needinfo?(sfoster)
> Hi Sam -
> 
> Partner is asking for the fix of this issue on 2.0, do you think it is
> possible you also land the fix on 2.0 as well?

We dont yet have any kind of fix at all, but I'll look at this today and we'll have to see what we can do.
Flags: needinfo?(sfoster)
Target Milestone: --- → 2.1 S5 (26sep)
Assignee: nobody → sfoster
Can QA reproduce on 2.0 as well?
blocking-b2g: 2.1? → 2.1+
Keywords: qawanted
Terrible UX bug.
blocker justification: poor user experience. Already requested for 2.0, blocking for 2.1
QA Contact: aalldredge
I was able to reproduce this issue on 2.0 Flame.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140916091857
Gaia: 95314e27da38b8c4edf965a77eded16cc5729ea8
Gecko: 5ecb32d79d03
Version: 32.0 (2.0)
Firmware: v165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0: --- → affected
Flags: needinfo?(jmitchell)
Keywords: qawanted
I've not been able to reproduce this on master anymore. Handing over to Aus who landed bug 1044125 which may have fixed, or is certainly related.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
(Assignee)

Comment 21

4 years ago
Will take a look tomorrow, hopefully when 1044125 lands in 2.1 it will also take care of this issue (like it does on master)
Assignee: sfoster → aus
Status: NEW → ASSIGNED
(Assignee)

Comment 22

4 years ago
I have confirmed that the patch for bug 1044125 fixes this issue. It should be landing shortly.
Depends on: 1044125
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1057300

Updated

4 years ago
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
(In reply to Ghislain Aus Lacroix [:aus] from comment #22)
> I have confirmed that the patch for bug 1044125 fixes this issue. It should
> be landing shortly.
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #24)
> (In reply to Ghislain Aus Lacroix [:aus] from comment #22)
> > I have confirmed that the patch for bug 1044125 fixes this issue. It should
> > be landing shortly.

Hi Ghislain -

Is it possible to land the patch of 1044125 to 2.0?

Thanks

Vance
Flags: needinfo?(aus)
(Assignee)

Comment 26

4 years ago
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #25)
> (In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #24)
> > (In reply to Ghislain Aus Lacroix [:aus] from comment #22)
> > > I have confirmed that the patch for bug 1044125 fixes this issue. It should
> > > be landing shortly.
> 
> Hi Ghislain -
> 
> Is it possible to land the patch of 1044125 to 2.0?
> 
> Thanks
> 
> Vance

It's unfortunately not possible to land it as is on 2.0, there are simply too many conflicts. I believe the fix would be similar, but it would require reimplementation.
Flags: needinfo?(aus)
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #13)
> [Blocking Requested - why for this release]:
> 
> Suggest to nominate to 2.1 blocker since the black screen on the card view
> will leave bad user experience to end users, also the RST is reasonable. So
> suggest to nominate to 2.1. Also originally partner ask this for 2.0
> blocker, so fixing this one in 2.1 give partner a chance to cheery-pick it
> to their code base
> 
> Thanks
> 
> Vance

Vance, please, lets avoid too-much cherry picking as that leads to fragmentation of our FxOS code as every partner will have a very different experience for the same version of FxOS. We should not be promoting this option at all, unless its a ship-stopper.
any idea if this is a Sam/aus is this a 2.0 regression ?

Adding qawanted for branch checks here.

Updated

4 years ago
Keywords: qawanted
(Assignee)

Comment 29

4 years ago
(In reply to bhavana bajaj [:bajaj] from comment #28)
> any idea if this is a Sam/aus is this a 2.0 regression ?
> 
> Adding qawanted for branch checks here.

afaik this bug was always present in 2.0 but intermittent. 

the fix itself should be similar as I mentioned in a previous comment and it feels reasonable to attempt it. we have many unit tests covering this stuff so we should be able to avoid regressions.

going to leave myself assigned to this for the 2.0 fix.

we can, however, go ahead and verify this on 2.2 and 2.1.
status-b2g-v2.0M: --- → affected
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
Created attachment 8493221 [details]
Card_View.png

Issue still repros on 2.1 Flame KK build 319MB.

The 'Settings' Ringtone section appears black when in Card View..

Issue still repros on 2.2 Flame KK build 319MB.

The 'Settings' Ringtone section appears white, however there is no text, the only difference is white vs black card view of said app.

I have uploaded a screenshot called Card_View

Environmental Variables:
Device: Flame 2.1
BuildID: 20140922080342
Gaia: 689c4ad4d8c3e4aa95805a2e49ae6cf786a1ae91
Gecko: c1c7a4eed459
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140922040649
Gaia: 3802009e1ab6c3ddfc3eb15522e3140a96b33336
Gecko: 5e704397529b
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
Flags: needinfo?(jmitchell)

Updated

4 years ago
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Keywords: qawanted
This issue is still present on 2.1 and 2.2, flipping flags to affected.
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
status-b2g-v2.1: fixed → affected
status-b2g-v2.2: fixed → affected
Flags: needinfo?(jmitchell)
QA Contact: aalldredge
(Assignee)

Comment 32

4 years ago
Darn, two birds with one stone fails. I'm trying to figure out what's going on with this today...
(Assignee)

Comment 33

4 years ago
So, I think this was not tested properly.
(Assignee)

Comment 34

4 years ago
(In reply to Ghislain Aus Lacroix [:aus] from comment #33)
> So, I think this was not tested properly.

Whoops, sorry, hit submit too fast. :) 

So, the video shows a slightly different problem (which I know, isn't obvious to the user, they still think it's broken the same way). This particular interaction triggers an activity window to be shown on top of the normal app frame. The getScreenshot method doesn't understand how to deal with this correctly and so we end up with a screenshot that looks blank because the underlying app isn't showing any content anymore.

I think we should attempt to verify that the original issue is gone in 2.2 and 2.1 and I can come up with a solution for this edgier case.
(Assignee)

Comment 35

4 years ago
QAWanted, see comment #34.
Keywords: qawanted
(Assignee)

Comment 36

4 years ago
Created attachment 8493392 [details] [review]
Pull Request (v2.2, v2.1) - Check for frontWindow in getScreenshot

Fairly easy to add a unit test as well for this once I know the patch is acceptable.
Attachment #8493392 - Flags: review?(alive)
(Assignee)

Updated

4 years ago
Attachment #8493392 - Attachment description: Pull Request - Check for frontWindow in getScreenshot → Pull Request (Master, v2.1) - Check for frontWindow in getScreenshot
Comment on attachment 8493392 [details] [review]
Pull Request (v2.2, v2.1) - Check for frontWindow in getScreenshot

Hi aus - the direction is correct but I will worry about if sometimes we really want to get the screenshot of the bottom window.

At least we should do

if (this.frontWindow && this.frontWindow.isActive() {
  this.frontWindow.getScreenshot();
  return;
}

But I am thinking: how do we trigger screenshot?
Is it because entering task manager will do AppWindow.prototype.close() ?
If so, AppWindow.prototype.close should trigger AppWindow.prototype.setVisible(false, true) when the closing animation terminates in AppTransitionController.prototype._enter_closed, and setVisible will trigger this.frontWindow.setVisible(false, true); again.

Could you investigate tell me why this path is failed? Thanks for help.
Attachment #8493392 - Flags: review?(alive)
(Assignee)

Comment 38

4 years ago
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #37)
> Comment on attachment 8493392 [details] [review]
> Pull Request (Master, v2.1) - Check for frontWindow in getScreenshot
> 
> Hi aus - the direction is correct but I will worry about if sometimes we
> really want to get the screenshot of the bottom window.
> 
> At least we should do
> 
> if (this.frontWindow && this.frontWindow.isActive() {
>   this.frontWindow.getScreenshot();
>   return;
> }

Ah, yes, I'll add a check to ensure if we have a frontWindow that it is active as well.

> 
> But I am thinking: how do we trigger screenshot?
> Is it because entering task manager will do AppWindow.prototype.close() ?
> If so, AppWindow.prototype.close should trigger
> AppWindow.prototype.setVisible(false, true) when the closing animation
> terminates in AppTransitionController.prototype._enter_closed, and
> setVisible will trigger this.frontWindow.setVisible(false, true); again.

The patch I landed for bug 1044125 decouples screenshots from visibility of the frame. We used to handle this in setVisible before my patch. We would check for a frontWindow and call it's setVisible (which we still do, but it only toggles frame visibility). I ended up regressing this functionality I think by forgetting to update getScreenshot to be aware of the frontWindow.

We now manage the screenshot process itself by waiting for the _closed event on the appwindow (that triggers getScreenshot) and we *show* the overlay with the screenshot when we get _sheetsgesturebegin (or _cardsviewbeforeshow) and hide on _sheetsgestureend (or _cardsviewclosed).

This fix is likely to end up in the patch for 2.0 as well along with select bits from the patch for bug 1044125.
(Assignee)

Comment 39

4 years ago
Comment on attachment 8493392 [details] [review]
Pull Request (v2.2, v2.1) - Check for frontWindow in getScreenshot

Updated PR with isActive check on front window and a unit test.
Attachment #8493392 - Flags: review?(alive)
(Assignee)

Updated

4 years ago
Attachment #8493392 - Attachment description: Pull Request (Master, v2.1) - Check for frontWindow in getScreenshot → Pull Request (v2.2, v2.1) - Check for frontWindow in getScreenshot
(Assignee)

Updated

4 years ago
Blocks: 1071295
(Reporter)

Comment 40

4 years ago
Hi Aus

I tested the patch under review. I still found issues.
Open two activities at a time & long press home button (enter card view)
1. See the front activity
2. see the back activity.
One of the activity is shown in black. I think one of the activity closes automically.

Please test it once or i will upload a video.

Thanks
Flags: needinfo?(aus)
Comment on attachment 8493392 [details] [review]
Pull Request (v2.2, v2.1) - Check for frontWindow in getScreenshot

Thanks, great!
Attachment #8493392 - Flags: review?(alive) → review+
(Assignee)

Comment 42

4 years ago
(In reply to ankit93040 from comment #40)
> Hi Aus
> 
> I tested the patch under review. I still found issues.
> Open two activities at a time & long press home button (enter card view)
> 1. See the front activity
> 2. see the back activity.
> One of the activity is shown in black. I think one of the activity closes
> automically.
> 
> Please test it once or i will upload a video.
> 
> Thanks

Ankit, I'm not sure what you're saying. I tried opening an activity on top of an application (that works as expected) then I tried to open two apps with an activity each on top of their windows and that works fine as well. If the activity is *closing* when we switch to cards view then this it is a different problem that should be filed separately.
Flags: needinfo?(aus)
(Assignee)

Comment 43

4 years ago
Comment on attachment 8493392 [details] [review]
Pull Request (v2.2, v2.1) - Check for frontWindow in getScreenshot

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): ?
[User impact] if declined: Activities on top of application windows won't be shown in the cards view. This doesn't reflect the actual state of the application.
[Testing completed]: On Flame (1G, 319M)
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None
Attachment #8493392 - Flags: approval-gaia-v2.1?(bbajaj)
(Assignee)

Updated

4 years ago
Blocks: 1072435
(Assignee)

Comment 44

4 years ago
Ankit, I filed a follow-up for the rest.

Fixed (master): https://github.com/mozilla-b2g/gaia/commit/096bee4ee9f3e21b9cdaa6c03b5a50e70c3c58c6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Whiteboard: [g+][LibGLA,TD94569,QE1, B] [systemsfe][LibGLA,TD95653,WW, B] → [g+][LibGLA,TD94569,QE1, B] [systemsfe][p=3][LibGLA,TD95653,WW, B]
(Assignee)

Comment 45

4 years ago
Crud, this is the one that needs uplift to 2.1 and a specific branch patch for 2.0. I'm marking fixed anyway to go through the normal uplift to 2.1 process. Just note that there will be a 2.0 specific patch that combines elements from bug 1044125 and this patch for 2.0.
status-b2g-v2.2: affected → fixed
(Assignee)

Comment 46

4 years ago
Created attachment 8494804 [details] [review]
Pull Request (v2.0) - Event driven screenshot overlays.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: Blank screenshot in Task Manager.
[Testing completed]: On Device (Flame, 319M)
[Risk to taking this patch] (and alternatives if risky): Medium
[String changes made]: None
Attachment #8494804 - Flags: review?(etienne)
Attachment #8494804 - Flags: review?(alive)
Attachment #8494804 - Flags: approval-gaia-v2.0?(bbajaj)
I am confused.

bug 1044125 is not in v2.0 but you are going to take this and bug 1044125 together to v2.0,
and bug 1044125 has already 2 known regressions (this bug and bug 1071510) if you get bug 1044125 into v2.0 you will need to get approval for bug 1071510 too. And maybe some regressions we didn't find if there is a module using AppWindow.prototype.setVisible(false, true) but it is deprecated in bug 1044125.

I am blaming but suspect all of this will go out of control if uplifted to v2.0
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #47)
> I am confused.
> 
> bug 1044125 is not in v2.0 but you are going to take this and bug 1044125
> together to v2.0,
> and bug 1044125 has already 2 known regressions (this bug and bug 1071510)
> if you get bug 1044125 into v2.0 you will need to get approval for bug
> 1071510 too. And maybe some regressions we didn't find if there is a module
> using AppWindow.prototype.setVisible(false, true) but it is deprecated in
> bug 1044125.
> 
> I am blaming 
sorry, I am not blaming :)))
Comment on attachment 8494804 [details] [review]
Pull Request (v2.0) - Event driven screenshot overlays.

The patch has some unwanted parts.
Attachment #8494804 - Flags: review?(alive)
Comment on attachment 8494804 [details] [review]
Pull Request (v2.0) - Event driven screenshot overlays.

Looks like the 2.1 patch + a tweak in AppWindow#requestScreenShotURL could get us there without the risky uplift.

Or am I missing something?
Attachment #8494804 - Flags: review?(etienne)
(Assignee)

Comment 51

4 years ago
(In reply to Etienne Segonzac (:etienne) from comment #50)
> Comment on attachment 8494804 [details] [review]
> Pull Request (v2.0) - Event driven screenshot overlays.
> 
> Looks like the 2.1 patch + a tweak in AppWindow#requestScreenShotURL could
> get us there without the risky uplift.
> 
> Or am I missing something?

Etienne, this is essentially that 2.1 patch with a few tweaks.

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #48)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #47)
> > I am confused.
> > 
> > bug 1044125 is not in v2.0 but you are going to take this and bug 1044125
> > together to v2.0,
> > and bug 1044125 has already 2 known regressions (this bug and bug 1071510)
> > if you get bug 1044125 into v2.0 you will need to get approval for bug
> > 1071510 too. And maybe some regressions we didn't find if there is a module
> > using AppWindow.prototype.setVisible(false, true) but it is deprecated in
> > bug 1044125.

I definitely understand your concern there. This is happening because we have a partner request to fix this particular issue. We're taking a known fix that already works in this case. Although, I definitely think that a similar patch to bug 1044125 would probably be necessary to fix the issue (since it's a race condition).

I marked 1071510 as a dependent so that we land it in 2.1 and 2.0.
(Assignee)

Updated

4 years ago
Depends on: 1071510
(Assignee)

Comment 52

4 years ago
I've removed the unwanted bits from the pull request for 2.0 but we now have to resolve a performance issue reported in bug 1073009 which seems to be caused by bug 1044125 (which is in part included in this PR). So yeah, complications. Nothing impossible, obviously, but this is going to need a few more days of work.

Comment 53

4 years ago
(In reply to Ghislain Aus Lacroix [:aus] from comment #35)
> QAWanted, see comment #34.

This issue does NOT repro on Flame 2.2
Actual Result: Card view displays app preview

Device: Flame 2.2
BuildID: 20140925053746
Gaia: c5d2e2f4ebf5f370d6003517057dcd47493dec90
Gecko: e9e56750ca5b
Version: 35.0a1 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0


This issue DOES repro on Flame 2.1 
Actual Result: Card view displays a black screen 

Device: Flame 2.1
BuildID: 20140925092639
Gaia: 86905e14c3ff06a0e6952ba635b6066ad2eea6b4
Gecko: e128c7d918de
Version: 34.0a2 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(jmitchell)
(Assignee)

Comment 54

4 years ago
(In reply to Seth Miko [:SethM] from comment #53)
> (In reply to Ghislain Aus Lacroix [:aus] from comment #35)
> > QAWanted, see comment #34.
> 
> This issue does NOT repro on Flame 2.2
> Actual Result: Card view displays app preview

Good! I landed the fix for that on master so I would have expected it to be fixed. :)

> This issue DOES repro on Flame 2.1 
> Actual Result: Card view displays a black screen 
> 
> Device: Flame 2.1
> BuildID: 20140925092639
> Gaia: 86905e14c3ff06a0e6952ba635b6066ad2eea6b4
> Gecko: e128c7d918de
> Version: 34.0a2 
> Firmware Version: L1TC10011800
> User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Yep! Waiting for approval to land there.

Thanks for double confirmation, I tested it myself but I wanted to make sure it worked before it got merged into 2.1.

Updated

4 years ago
Attachment #8493392 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+

Updated

4 years ago
blocking-b2g: 2.1+ → 2.0+
Comment on attachment 8494804 [details] [review]
Pull Request (v2.0) - Event driven screenshot overlays.

Adding verifyme to make sure this is verified across all branches post landing.
Attachment #8494804 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+

Updated

4 years ago
Keywords: verifyme
v2.1: https://github.com/mozilla-b2g/gaia/commit/15858d7a5fe4c190b24399fd7cf9c2548ccfdf98

Looks like the v2.0 Gaia-Try run has test failures? At least Gu looks suspicious.
https://tbpl.mozilla.org/?rev=5022264a76e880fa352cae16ba5f6d51a0f07400&tree=Gaia-Try
status-b2g-v2.1: affected → fixed
Flags: needinfo?(aus)
(Assignee)

Comment 58

4 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #57)
> v2.1:
> https://github.com/mozilla-b2g/gaia/commit/
> 15858d7a5fe4c190b24399fd7cf9c2548ccfdf98
> 
> Looks like the v2.0 Gaia-Try run has test failures? At least Gu looks
> suspicious.
> https://tbpl.mozilla.org/
> ?rev=5022264a76e880fa352cae16ba5f6d51a0f07400&tree=Gaia-Try
Blocks: 1069452
Flags: needinfo?(aus)
(Assignee)

Comment 59

4 years ago
I'm updating the test now, will wait for it to be green before pushing to 2.0. Also, it should be noted that performance will regress momentarily as we wait to get approval to land bug 1069452 on 2.1 + 2.0.
(Assignee)

Comment 60

4 years ago
Commit (v2.0): https://github.com/mozilla-b2g/gaia/commit/5c2303ec4e367da060aa1b807d541a6549b3d72a

Fixed on 2.0.
status-b2g-v2.0: affected → fixed
Whiteboard: [g+][LibGLA,TD94569,QE1, B] [systemsfe][p=3][LibGLA,TD95653,WW, B] → [g+][LibGLA,TD94569,QE1, B] [systemsfe][p=8][LibGLA,TD95653,WW, B]
(Assignee)

Comment 61

4 years ago
Merge conflict for 2.0m need to be manually resolved. Doing that now.
(Assignee)

Comment 63

4 years ago
False start on the first one... updated try run here: https://tbpl.mozilla.org/?rev=60ceb4a9f1813d22c0232cc2ff140b5ee42a1bab&tree=Gaia-Try

This will get merged on 2.0m today. :)
(Assignee)

Comment 64

4 years ago
Commit (v2.0m): https://github.com/mozilla-b2g/gaia/commit/f86fd5c55255b1c8b18dfc124ec0d0ec1d4d8fb6

Fixed on 2.0m. This can now be tested on all branches. Flagging QAWanted for extra visibility. :)
status-b2g-v2.0M: affected → fixed
Keywords: qawanted
Verified fixed on Flame KK 2.0, 2.1, 2.2(Master) -   QA-wanted team does not have a Madia device to test 2.0M on


Verified against STR from comment 8 and STR from comment 10

Actual Results - card views appeared normal (screenshots and icons)
status-b2g-v2.0: fixed → verified
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
Keywords: qawanted
(Assignee)

Updated

4 years ago
Blocks: 1073392
Status: RESOLVED → VERIFIED

Updated

4 years ago
Keywords: verifyme
(Reporter)

Comment 66

4 years ago
Created attachment 8500392 [details]
lockscreen.zip

Hi Aus

There is a sideeffect of this patch.

Put the above uploaded (unzipped) app in Gaia/apps folder.
Go to settings->sound. Press home button.
Launch the Lockscreens app from homescreen.

It automatically closes.

Thanks
Flags: needinfo?(aus)
(Reporter)

Comment 67

4 years ago
Hi Alive

Please see comment#66. I have uploaded a zipped app here. If you see the code in main.js file of that app - 
If I use window.close in setTimeout the issue doesn't happens.

Need your suggestion on this. 

Issue is reproducible on Mozilla v2.0. I didn't see it on Master.
Flags: needinfo?(alive)
(In reply to ankit93040 from comment #67)
> Hi Alive
> 
> Please see comment#66. I have uploaded a zipped app here. If you see the
> code in main.js file of that app - 
> If I use window.close in setTimeout the issue doesn't happens.
> 
> Need your suggestion on this. 
> 
> Issue is reproducible on Mozilla v2.0. I didn't see it on Master.

Well, interesting.
But I am able to repro on master with your app. I don't think it's related to this patch.
Please open another bug.
Flags: needinfo?(alive)
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #68)
> (In reply to ankit93040 from comment #67)
> > Hi Alive
> > 
> > Please see comment#66. I have uploaded a zipped app here. If you see the
> > code in main.js file of that app - 
> > If I use window.close in setTimeout the issue doesn't happens.
> > 
> > Need your suggestion on this. 
> > 
> > Issue is reproducible on Mozilla v2.0. I didn't see it on Master.
> 
> Well, interesting.
> But I am able to repro on master with your app. I don't think it's related
> to this patch.
> Please open another bug.

I checked your code and found there's a race in AWM. It may not be easy to fix.
I'll suggest to do
```js
window.addEventListener('visibilitychange', function() {
  window.close();
});
```
in your code, instead of activity onsuccess/onerror callback.
(Reporter)

Comment 70

4 years ago
Alive, 

As requested I have made a bug https://bugzilla.mozilla.org/show_bug.cgi?id=1079014
I am not able to needinfo you there don't know why. I have also added Aus@mozilla.com.

We can't fix it for one sceanrio as in there may be third party app which might be using this way. So we need to resolve these in system app.

It's a blocker for us. Request you to please see the new raised bug.

Thanks
(In reply to ankit93040 from comment #70)
> Alive, 
> 
> As requested I have made a bug
> https://bugzilla.mozilla.org/show_bug.cgi?id=1079014
> I am not able to needinfo you there don't know why. I have also added
> Aus@mozilla.com.
> 
> We can't fix it for one sceanrio as in there may be third party app which
> might be using this way. So we need to resolve these in system app.

Why and who is telling the 3rd party apps to code something inaccurate?
Using visibilitychange event is something better than what you have.
(Assignee)

Comment 72

4 years ago
Let's move the conversation to the new bug :) My patch is not causing the issue. If I back-out my patch locally I can still reproduce.
Flags: needinfo?(aus)

Comment 73

4 years ago
Verify passed, this issue can't be repro on Woodduck 2.0.
Attached: Verify_Woodduck_Ringtones.mp4
Reproducing rate: 0/5
Gaia-Rev        ee5cf148b4c546beea9bfb799d2a3ee62074957d
Gecko-Rev       73601b71861cbc2f180c4d2653cec3e9fbb39db5
Build-ID        20141114050313
Version         32.0
status-b2g-v2.0M: fixed → verified

Comment 74

4 years ago
Created attachment 8522809 [details]
Verify_Woodduck_Ringtones.MP4
You need to log in before you can comment on or make changes to this bug.