1047143 - [User Story] Tapping the home button while in the Task Switcher should return the user to the view from which Task Manager was launched

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Rob MacDonald [:robmac]]

As a user who is in the Task Switcher, I want a tap of the home button to return to the view from which the Task Switcher was launched.

Acceptance Criteria:
1) If while the Task Switcher is open, the user taps the home button, the Task Switcher will close and return to the view from which the Task Switcher was launched.

Comment 1

[Markus Nilsson]

Attachment: Github patch

Hi Francis.

Rob asked you to look at the patches for the other soft home button bugs, I hope you can take a look at this one as well :)

Comment 2

[Francis Djabri [:djabber]]

Thanks Markus. This meets the acceptance criteria. When we update the Task manager design to a flat style, we will have certain animation requirements in place (see https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) but this works for the current task manager.

Comment 3

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8466932 [details] [review]
Github patch

Redirecting to Alive for owner review. Also, Sam Foster is going to have a look and give feedback since he has been working in the task manager lately.

Comment 4

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8466932 [details] [review]
Github patch

Drive-by comments: I applied the patch and tried it out: looks good and works well. I left a note in GH and I'll note here as a reminder to myself mostly that we'll need to revisit the logic here when the homescreen gets added to the task switcher in bug 1047143.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

I don't think this works and I think UX doesn't give enough information here...sorry.

What we should do is:
1. TaskManager should block the holdhome and home events.
2. AppWindowManager should remove the holdhome event listener.
3. VisibilityManager should listen to taskmanageropened event to dispatch hidewindow event for AWM.
4. What if the last app is killed during task manager?
5. What's the animation when we hold the home button if there is an opened app?
6. What's the animation when we press the home button in task manager when the app before showing task manager is not homescreen?
7. What's the animation when we press the home button in task manager when the app before showing task manager is homescreen?
8. Do we expect all app in task manager are background?

Guys, this is a big change. don't rush like that.. remember the last app in task manager is something bad :/

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Francis Djabri [:djabber] from comment #2)
> Thanks Markus. This meets the acceptance criteria. When we update the Task
> manager design to a flat style, we will have certain animation requirements
> in place (see
> https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) but this
> works for the current task manager.

Please note how you define the animation is important for how we develop.
This may involve some architecture change or hierarchy change.

Comment 7

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #5)
> I don't think this works and I think UX doesn't give enough information
> here...sorry.
> 
> What we should do is:
> 4. What if the last app is killed during task manager?
> 5. What's the animation when we hold the home button if there is an opened
> app?
> 6. What's the animation when we press the home button in task manager when
> the app before showing task manager is not homescreen?
> 7. What's the animation when we press the home button in task manager when
> the app before showing task manager is homescreen?
> 8. Do we expect all app in task manager are background?


Francis, do we have answers to these questions?

Comment 8

[Francis Djabri [:djabber]]

> > What we should do is:
> > 4. What if the last app is killed during task manager?
Please refer to the spec, under the Relaunching tasks section: https://mozilla.box.com/s/lbw2wzw3p4jvxs24k4sg. If an app is killed, it should be relaunched. 

> > 5. What's the animation when we hold the home button if there is an opened
> > app?
Please see the animation linked to above (https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) at 0:02 seconds

> > 6. What's the animation when we press the home button in task manager when
> > the app before showing task manager is not homescreen?
Please see the animation linked to above (https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) at 0:06 seconds

> > 7. What's the animation when we press the home button in task manager when
> > the app before showing task manager is homescreen?

This should follow the animation in 6. above, but flagging Eric to confirm.
 
> > 8. Do we expect all app in task manager are background?
> 
I'm afraid I don't follow the question - is this a question for UX? According to the spec, all apps should show a screenshot if one is available, even if the app has been quiesced or killed in the background.

Comment 9

[Eric Pang [:epang] UX]

Regarding #7.  Yes this is correct, also, the last app used will always be next to the homescreen.  Thanks Francis!

Comment 10

[Sam Foster [:sfoster] (he/him)]

Alive is asking about the state of the apps when we are in the task-manager, whether the current app should switch state from being active to being inactive/background when we enter the task-manager. Its an important question for both driving the transitions and will also determine if an app can be suspended or killed while in the task-manager.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Francis Djabri [:djabber] from comment #8)
> > > What we should do is:
> > > 4. What if the last app is killed during task manager?
> Please refer to the spec, under the Relaunching tasks section:
> https://mozilla.box.com/s/lbw2wzw3p4jvxs24k4sg. If an app is killed, it
> should be relaunched. 

We need more considerations about this.
* Do we want to revive all the apps in task manager while the task manager is opened and some of them are killed at background?
* Do we want to revive all the apps even while the task manager is not opened but some of them are killed at background?

> 
> > > 5. What's the animation when we hold the home button if there is an opened
> > > app?
> Please see the animation linked to above
> (https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) at 0:02
> seconds

Cannot access :/

> 
> > > 6. What's the animation when we press the home button in task manager when
> > > the app before showing task manager is not homescreen?
> Please see the animation linked to above
> (https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) at 0:06
> seconds
> 
> > > 7. What's the animation when we press the home button in task manager when
> > > the app before showing task manager is homescreen?
> 
> This should follow the animation in 6. above, but flagging Eric to confirm.
>  
> > > 8. Do we expect all app in task manager are background?
> > 
> I'm afraid I don't follow the question - is this a question for UX?
> According to the spec, all apps should show a screenshot if one is
> available, even if the app has been quiesced or killed in the background.

Comment 12

[Sam Foster [:sfoster] (he/him)]

Just a note to clarify what is probably already obvious: currently a part of showing the cards view involves switching to the homescreen and transitioning from there to cards view. That is, long-pressing on home changes what the current app is. 

This user story proposes changing that so that the new task switcher does not imply a change in the active app or your stack/history. Implementation-wise, this is an important change, which I don't yet fully understand the ramifications of. It may be as simple as just removing the code that switches us to the homescreen before showing the cards view. But I need to better understand if there were technical reasons why we did that in the first place? 

It seems to me we do need to put the current app into the background (or some other "paused"/inactive state) when showing the task strip. Otherwise the camera could stay on, and other bad things might happen. Can we have a state where *all* apps are in the background?    

As for reviving quiesced/killed apps, as I understand it we have kept the AppWindow instance, but the browser/iframe has been removed. For the purposes of representing apps in this state in the task switcher that should be fine - as long as we have the name and an icon or screenshot. We'll just need to revive the app before switching to it when the user selects that card. This would make the task switcher much more useful in low-memory environments where essentially every app is killed when it goes into the background. 

That said, we should tease these issues apart: representing the stack as-is today, and expanding on the stack to include recently killed apps are separate, orthogonal tasks. Blocking one on the other may put the feature at further risk for 2.1. Perhaps we need a new user story that explicitly states the behavior we want for recently killed/quiesced apps, in these terms?

Comment 13

[Francis Djabri [:djabber]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> (In reply to Francis Djabri [:djabber] from comment #8)
> > > > What we should do is:
> > > > 4. What if the last app is killed during task manager?
> > Please refer to the spec, under the Relaunching tasks section:
> > https://mozilla.box.com/s/lbw2wzw3p4jvxs24k4sg. If an app is killed, it
> > should be relaunched. 
> 
> We need more considerations about this.
> * Do we want to revive all the apps in task manager while the task manager
> is opened and some of them are killed at background?
> * Do we want to revive all the apps even while the task manager is not
> opened but some of them are killed at background?
> 

I'm not quite sure if I follow, but here are some comments that will hopefully clarify: 

We don't need to revive *all* the apps in the task manager, but we certainly need to have the ability to revive any application that is selected by the user. The task manager should show a task strip of historical apps/browser windows, whether they are active, in the background, or killed/quiesced. If the user selects a killed/quiesced app, then it should be revived. 

The task strip should include as long a list as possible, but it does not need to be endless.  Although I don’t see any need to limit the list from a UX perspective, I would think that anything above around 30 items wouldn’t provide much additional value, so I’d be ok putting in a limit at around this figure if that is of any help. 

> > 
> > > > 5. What's the animation when we hold the home button if there is an opened
> > > > app?
> > Please see the animation linked to above
> > (https://mozilla.app.box.com/files/0/f/2194656499/1/f_19215648115) at 0:02
> > seconds
> 
> Cannot access :/

I made access to Eric's animations public, so hopefully this should work now. 
> 
> >

Comment 14

[Francis Djabri [:djabber]]

After chatting with Sam, it seems that there are a few independent issues that are worth keeping track of:

1) The task manager should include a full historical list of apps, not just the active apps. 
2) The task manager should be able to revive apps that have been killed/quiesced.
3) The task manager should keep the current app active even if the user invokes the task manager, so that pressing home again should return the user to the app.
4) The task manager should adopt a new flat style that allows the user to scroll easily between a large range of apps

It would be great to know if any of these issues are problematic ahead of time so that if they are at risk we can prioritize them in a controlled way.

Comment 15

[Sam Foster [:sfoster] (he/him)]

I've decided to tackle this independent and ahead of trying to land the flat/haida-style task switcher. That maximises the time we'll have to work this through properly and lets us test one change at a time. This behavior change is orthoganol to the style changes and the interaction can work just as well with the existing carousel-style as the new flat scrolling style. 

Alive and I discussed this in some detail on irc last night, I'll cancel that ni? for now.

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Sam Foster [:sfoster] from comment #15)
> I've decided to tackle this independent and ahead of trying to land the
> flat/haida-style task switcher. That maximises the time we'll have to work
> this through properly and lets us test one change at a time. This behavior
> change is orthoganol to the style changes and the interaction can work just
> as well with the existing carousel-style as the new flat scrolling style. 
> 
> Alive and I discussed this in some detail on irc last night, I'll cancel
> that ni? for now.

What sam said.

Comment 17

[Sam Foster [:sfoster] (he/him)]

Attachment: close apps entering task-manager, exitToApp to open/restore

My plan right now is to get tests working and ask for r? So any feedback you have at this point would be useful. 

* I added a goPosition method to StackManager to handle the task switcher case - where we move to an arbitrary point in the stack vs. edge gestures back/next. 

* In the task manager, I channel all exit points through an exitToApp method. 

* The only straggler is when we have an attentionscreenshow event - not sure how to handle this, I can exitToApp() to restore the previous app but we might get into a confused state? 

I had another version using exitToApp but calling AppWindowManager.display() instead of the StackManager.goPosition(), but this way seems to be closer to the model we are shooting for - where the task-switcher is just a kind of zoomed out view on the sheets you swipe through with the edge gesture.

Comment 18

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Sam Foster [:sfoster] from comment #17)
> Created attachment 8474892 [details] [review]
> close apps entering task-manager, exitToApp to open/restore
> 
> My plan right now is to get tests working and ask for r? So any feedback you
> have at this point would be useful. 
> 
> * I added a goPosition method to StackManager to handle the task switcher
> case - where we move to an arbitrary point in the stack vs. edge gestures
> back/next. 

I doubt this: what if we only call open()?
I know we have some pain on updating StackManager and AppWindowManager, and my plan is to merge them in the future. If stackManager is not updated by app.open() let's fire a new event from taskManager to stackManager.

> 
> * In the task manager, I channel all exit points through an exitToApp
> method. 
> 
> * The only straggler is when we have an attentionscreenshow event - not sure
> how to handle this, I can exitToApp() to restore the previous app but we
> might get into a confused state? 

Right, the best solution is ask UX opinion. If we don't need to exit task manager?

But if yes, let's do this like what I did in bug 927862:
Implement AppWindow.prototype.requestForeground, replace it with this.app.setVisible(true) in appTransitionController.
VisibilityManager should deal with requestForeground and see if AttentionScreen/LockScreen is there.
If not, setVisible(true) to the app.

> 
> I had another version using exitToApp but calling AppWindowManager.display()
> instead of the StackManager.goPosition(), but this way seems to be closer to
> the model we are shooting for - where the task-switcher is just a kind of
> zoomed out view on the sheets you swipe through with the edge gesture.

Not sure what's the difference here..is it about animation?

Comment 19

[Sam Foster [:sfoster] (he/him)]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #18)
> (In reply to Sam Foster [:sfoster] from comment #17)
> > Created attachment 8474892 [details] [review]
> > close apps entering task-manager, exitToApp to open/restore
> > 
> > My plan right now is to get tests working and ask for r? So any feedback you
> > have at this point would be useful. 
> > 
> > * I added a goPosition method to StackManager to handle the task switcher
> > case - where we move to an arbitrary point in the stack vs. edge gestures
> > back/next. 
> 
> I doubt this: what if we only call open()?
> I know we have some pain on updating StackManager and AppWindowManager, and
> my plan is to merge them in the future. If stackManager is not updated by
> app.open() let's fire a new event from taskManager to stackManager.

Yeah there is an odd overlap of function between StackManager and AppWindowManager. I'll look into using app.open instead.

> > I had another version using exitToApp but calling AppWindowManager.display()
> > instead of the StackManager.goPosition(), but this way seems to be closer to
> > the model we are shooting for - where the task-switcher is just a kind of
> > zoomed out view on the sheets you swipe through with the edge gesture.
> 
> Not sure what's the difference here..is it about animation?

Its mostly about semantics I think, ultimately both approaches end up doing the same thing but I wanted to closely align the task switcher with edge gestures and make it functions of the stack manager. If I switch to using app.open I'll ensure the stack manager's state is kept in sync. I think the CSS hooks are there to implement this to spec either way.

Comment 20

[Sam Foster [:sfoster] (he/him)]

PR is updated to use appWindow.open(), plus new and fixed tests. This is ready for review except for 2 things: 

1) I forgot to chase up an answer to the question about attention screens: Rob, the effect of this patch is that we no longer make homescreen the active app when showing the task switcher. What should happen if there's an attention screen while the task switcher is showing? If we simply hide the task switcher, we'll be at the app that was active before showing it. We can switch to the homescreen and hide the task switcher. We may be able to show the attention screen over the top of the task switcher too...

2) Alive: At https://github.com/mozilla-b2g/gaia/pull/23011/files#diff-d0f4081aee928fe4cf6ad625c441dd9cR533 in closeApp I currently call AppWindowManager._updateActiveApp(homescreenLauncher.getHomescreen().instanceID); Calling the hidden method is probably a bad thing, should I raise a new event here do you think? 

Finally, I remembered one of the reasons I thought of going through the StackManager to accomplish the state change: AppWindow's handle__swipein does this.reviveBrowser(); I think we want that for the task switcher too - not necessarily in this bug though. Otherwise, the call to close/open seems to be working out OK. I think the transition changed though, as the appWindow is going from closed + in-task-manager, to opened. It looks fine to my (admittedly tired eyes) but that might need addressing here or in a follow-up.

Comment 21

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

Etienne, this is just the part of the 2.1 task manager behavior that says we should return to the current/active app when pressing 'home', which implies we don't open the homescreen when showing the task manager. It's the existing carousel style cards view otherwise, populated from StackManager.snapshot(). 

If we need the task manager to show child windows also, we probably want to modify StackManager.snapshot, or provide a new method on there. And that should be a separate bug - I don't think we have one filed yet.

Comment 22

[Sam Foster [:sfoster] (he/him)]

Rob, Francis and I discussed the attention screen question and decided that we should return to the active app. E.g.:

1) show the task switcher from app1, 
2) incoming call dismisses the task switcher to make app1 active, meanwhile it is overlaid by the incoming call window
3) I dismiss/complete the call and find myself back in app1.

Comment 23

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Sam Foster [:sfoster] from comment #20)
> PR is updated to use appWindow.open(), plus new and fixed tests. This is
> ready for review except for 2 things: 
> 
> 1) I forgot to chase up an answer to the question about attention screens:
> Rob, the effect of this patch is that we no longer make homescreen the
> active app when showing the task switcher. What should happen if there's an
> attention screen while the task switcher is showing? If we simply hide the
> task switcher, we'll be at the app that was active before showing it. We can
> switch to the homescreen and hide the task switcher. We may be able to show
> the attention screen over the top of the task switcher too...
> 
> 2) Alive: At
> https://github.com/mozilla-b2g/gaia/pull/23011/files#diff-
> d0f4081aee928fe4cf6ad625c441dd9cR533 in closeApp I currently call
> AppWindowManager._updateActiveApp(homescreenLauncher.getHomescreen().
> instanceID); Calling the hidden method is probably a bad thing, should I
> raise a new event here do you think? 

Works for me. TaskManager will become a submodule of appWindowManager later.

> 
> Finally, I remembered one of the reasons I thought of going through the
> StackManager to accomplish the state change: AppWindow's handle__swipein
> does this.reviveBrowser(); I think we want that for the task switcher too -
> not necessarily in this bug though.

What we need is turn on "suspending-app.enabled" always..
Though the UX doesn't think reviving all tasks are necessary,
but I am curious about this scenario: if I just open a game app and email app, and the game app is killed before entering task manager, I still expect to see it because it's just fresh new in my memory.
Only reviving app from task manager is something doesn't make sense.

Otherwise, the call to close/open seems
> to be working out OK. I think the transition changed though, as the
> appWindow is going from closed + in-task-manager, to opened. It looks fine
> to my (admittedly tired eyes) but that might need addressing here or in a
> follow-up.

Yes.

Comment 24

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

The reason of overlapping of StackManager and AppWindowManager is simple:
StackManager is invented before bug 907013 so there was not AppWindowManager then.

What's in my mind is we need to combine StackManager with AppWindowManager(as a submodule) so we don't need to maintain two lists.

Comment 25

[Etienne Segonzac (:etienne)]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

When an app is displayed, triggering the card view then going back to the same app (by tapping it) works properly.

But triggering the cardview then pressing home to come back to the app puts this app on top of the stack, which is not okay. The cardview, like the edge gestures, should never modify the order of the stack.

Your earlier approach with |goToPosition| probably did that properly... or we can use the |cardviewclosed| event.
I know it's a pain for everybody, but there's no way around the StackManager for cardview/edge gestures related features :/

Also, the |.in-task-manager| css class removes the transform, which causes the current app to "flash" every time you open the cardview (removing the transform puts the appWindow into the viewport). In |enterTaskManager|, when the app |isActive()|, you might want to wait for the internal _closed event before adding the |.in-task-manager| css class.

Comment 26

[Sam Foster [:sfoster] (he/him)]

I've updated the PR, addressing a number of comments: 

* Now tracks the StackManager properly, not modifying the order of the stack
* Fix an issue in StackManager._remove when removing the last app - it now sets StackManager.position to -1, whic is correctly handled by TaskManager to exit to homescreen
* Fix transition issues - I just removed that .in-task-manager rule which was overriding the transform. We see a glimpse of the homescreen when showing the task-switcher from another app, but I think it looks ok? Is the same as what we have previous to this patch. 
* Implemented AppWindow.prototype.requestForeground and the apprequestforeground handler in VisibilityManager. I've left logging in around this area because this still isn't working correctly for me: the VisibilityManager correctly refuses to setVisible(true) on the app requesting foreground when the attention screen is showing, but for a reason I've not spotted yet, the app gets foreground-ed anyway. 

I think that's the last issue here. Maybe fresh eyes will see the error. Any feedback appreciated.

Comment 27

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

Either or both of you :)

Comment 28

[Sam Foster [:sfoster] (he/him)]

I spent a while troubleshooting the attentionscreen problem. It turns out that AttentionScreen listens for 'appwillopen' events, and handles them by hiding itself. The requestForeground method works great for foreground/background, appwillopen is fired by the appWindow.transitionController when we open(). 

I'm reluctant to change much around the AttentionScreen as I don't know what impact those changes will have at present. 

I spent a little while looking into having a side-effect-free task switcher which leaves state as-is (i.e. doesn't close appWindows when it shows), but that only defers the problem, you still have to transition the current and selected app - so no net gain. 

I also tried listening for attentionscreenhide to defer re-opening the current app until the attentionscreen went away, but that looked very awkward as the dialer screen disapeared leaving black, then the previous/current app faded in. Maybe that is the right idea though and I just need to improve this transition. 

I'm all ears for suggestions.

Comment 29

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

Sorry for late feedback. I think we are near the final patch. f+ with nits

Comment 30

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

As dicussed in IRC:
* I removed the appwillopen handler in AttentionScreen, it is already listening for launchapp and conditionally hiding based on stayBackground. 
* Changed the events SoundManager listens for to homescreenopening and homescreenopened - which it uses to determine if we're on the homescreen. 
* Also fixed up the app_window unit tests so they know about requestForeground.

Comment 31

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

Manually testing various scenarios with opening and dimissing the task switcher looks good so far. I also ran all the #system unit tests locally and they came up green. So I think this is ready for a code-review pass and ui-review as well, with the understanding that ui-review- may necessitate a round of changes and re-review.

Comment 32

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

I left some comments, lemme know if you want me to look at certain parts if you fixed them.

Comment 33

[Sam Foster [:sfoster] (he/him)]

New unit tests added, clean up as requested. Waiting for Gaia-Try results: https://tbpl.mozilla.org/?rev=f381bccc3dcba8cc2f7139c5b89cc8d957055118&tree=Gaia-Try

Comment 34

[Rob MacDonald [:robmac]]

Comment on attachment 8474892 [details] [review]
close apps entering task-manager, exitToApp to open/restore

Reviewed using the existing carousel task switcher. Seems to work as specified.

Comment 35

[Sam Foster [:sfoster] (he/him)]

Gaia-Try: https://tbpl.mozilla.org/?rev=f381bccc3dcba8cc2f7139c5b89cc8d957055118&tree=Gaia-Try

I've retriggered the marionette tests, as tucked inbetween the intermittents, there's: 

TEST-UNEXPECTED-FAIL | /builds/slave/test/gaia/apps/system/test/marionette/camera_on_call_test.js | Camera app on call should be cleared after opening app

As this has to do with home events and attentionscreen it seems plausible this is a legit breakage. I've re-triggered though to be sure. I cant follow exactly what the test is testing, or how to reproduce manually to verify. Thoughts?

Comment 36

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Sam Foster [:sfoster] from comment #35)
> Gaia-Try:
> https://tbpl.mozilla.org/
> ?rev=f381bccc3dcba8cc2f7139c5b89cc8d957055118&tree=Gaia-Try
> 
> I've retriggered the marionette tests, as tucked inbetween the
> intermittents, there's: 
> 
> TEST-UNEXPECTED-FAIL |
> /builds/slave/test/gaia/apps/system/test/marionette/camera_on_call_test.js |
> Camera app on call should be cleared after opening app
> 
> As this has to do with home events and attentionscreen it seems plausible
> this is a legit breakage. I've re-triggered though to be sure. I cant follow
> exactly what the test is testing, or how to reproduce manually to verify.
> Thoughts?

Are you able to reproduce it manually? I am going to remove this test when bug 927862 lands.
And I have difficulty to see what happens now, so if you are not able to repro, disable it.

Comment 37

[Sam Foster [:sfoster] (he/him)]

Actually yes I can reproduce this. The test has you open an app - I used Settings, get an attention screen (I used an alarm) and press home to go back to the homescreen while that attentionscreen is up. If I try to re-launch Settings by clicking on its icon from the homescreen, it comes up but the browser is hidden and the screenshot is visible. It has the active class, and transition-state is opened, but because of the attentionscreen, requestForeground does not result in the setVisible(true) happening. 

I'm not sure yet how this is supposed to work. The AttentionScreen has gone into its notification-bar state once we tapped to go back to the homescreen, so ISTM we should allow Settings to setVisible here. Removing the willopen handler has caused this I guess. Maybe in AttentionScreen I should be testing AttentionScreen.isFullyVisible() rather than isVisible()? 

I can try this later, it sounds right. But if this is true and this is the only test to catch it, I would feel a lot better with the confirmation of someone who knows this code better than I.

Comment 38

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Sam Foster [:sfoster] from comment #37)
> Actually yes I can reproduce this. The test has you open an app - I used
> Settings, get an attention screen (I used an alarm) and press home to go
> back to the homescreen while that attentionscreen is up. If I try to
> re-launch Settings by clicking on its icon from the homescreen, it comes up
> but the browser is hidden and the screenshot is visible. It has the active
> class, and transition-state is opened, but because of the attentionscreen,
> requestForeground does not result in the setVisible(true) happening. 
> 
> I'm not sure yet how this is supposed to work. The AttentionScreen has gone
> into its notification-bar state once we tapped to go back to the homescreen,
> so ISTM we should allow Settings to setVisible here. Removing the willopen
> handler has caused this I guess. Maybe in AttentionScreen I should be
> testing AttentionScreen.isFullyVisible() rather than isVisible()? 

You are right! My fault not catching this!
https://github.com/mozilla-b2g/gaia/pull/23011/files#diff-6e0622e9bb45439a918e53af81c10fafR64

Comment 39

[Sam Foster [:sfoster] (he/him)]

Merged into master: https://github.com/mozilla-b2g/gaia/commit/ea93363a8c424d65a9ad91438ce6961377a20f98
Commit: https://github.com/mozilla-b2g/gaia/commit/1d9a8cf867f281d7ab22402f246bac48d0980c33

Comment 40

[Kevin Grandon :kgrandon]

This is causing multiple marionetteJS failures since landing: https://tbpl.mozilla.org/php/getParsedLog.php?id=46780659&tree=B2g-Inbound

Comment 41

[Kevin Grandon :kgrandon]

Backed out for causing Gij failures: https://github.com/mozilla-b2g/gaia/commit/91d744b1b86881d5a39a5bda553413c8f4d2753a

I will help investigate this with you later.

Comment 42

[Sam Foster [:sfoster] (he/him)]

Fixed marionette test, Gaia-Try is green (linter error not mine!)
https://tbpl.mozilla.org/?rev=ab85c88016c5fb232addf1b2cc82fe19943437be&tree=Gaia-Try 

Merged: https://github.com/mozilla-b2g/gaia/commit/c4284d24db251582731424617d268394b9045310
Commit: https://github.com/mozilla-b2g/gaia/commit/450be0707d124ad06f15cd0dda1dd03ce2051fa6

Comment 43

[Carsten Book [:Tomcat]]

sorry had to revert this change in https://github.com/mozilla-b2g/gaia/commit/52c3473fbe58b16e8729e11cb5a61078a93166cc since this caused leaks on b2g-inbound like

https://tbpl.mozilla.org/php/getParsedLog.php?id=46828873&tree=B2g-Inbound

Comment 44

[(inactive after 6/18) Alive Kuo [:alive]]

Reopen to reflect the revert.

Comment 45

[Sam Foster [:sfoster] (he/him)]

Nice green try: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=2afea88c19fd
...Aaaand landed again: https://github.com/mozilla-b2g/gaia/commit/b34143e3b9edce351159eff495b5e41695ef861d

Comment 46

[Edward Chen[:edchen]]

Test cases:
https://moztrap.mozilla.org/manage/cases/?pagenumber=1&pagesize=100&sortfield=created_on&sortdirection=desc&filter-tag=3018

Comment 47

[Edward Chen[:edchen]]

Hi Seth,

I does not see any task switcher feature landed in v2.1, but you was mark ticket to "VERIFIED". Could you double confirm this status.

Thanks,
Edward

Comment 48

[Seth Miko [:SMiko]]

This issue is verified fixed on Flame 2.1(319mb)

Flame 2.1(319mb)

Environmental Variables:
Device: Flame Master (319mb)
BuildID: 20140902040205
Gaia: 44bf2e3bc5ddea9db9a8c851bd353cb234aa883c
Gecko: c360f3d1c00d
Version: 34.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

STR:
1: Launch any app (ex. Settings).
2: Hold home button to enter the Task Switcher.
3: Navigate between cards (open apps)
4: Tap home button

Actual Result: The user is returned to the app in which the Task Switcher was launched (ex. Settings)

Comment 49

[Seth Miko [:SMiko]]

The task manager behaves as described in https://mozilla.box.com/s/lbw2wzw3p4jvxs24k4sg. (from comment 8)
However, there are two features that are not present; landscape mode, and Add to Home

Comment 52

[Sam Foster [:sfoster] (he/him)]

Note that although this did land, as the other parts of the revised task manager did not make it to 2.1, we eventually made the decision to reverse this to match the behavior to match 2.0, which was implemented in bug 1071852.