Gaia part of bug 798445: Remove |activity.onsuccess = reopenApp| callbacks.

VERIFIED FIXED in B2G C2 (20nov-10dec)

Status

P3
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: timdream, Assigned: alive)

Tracking

unspecified
B2G C2 (20nov-10dec)
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(1 attachment)

When both bug 798445 and bug 799039 landz, apps, doesn't need to, and should not attempt to, bring themselves to the foreground since all switching is done entirely by the System app.

The patch of this bug should do what bug 799039 comment 0 item 4 says.
Will work on this.
Status: NEW → ASSIGNED
QA Contact: ttaubert
Blocks: 802643
No longer blocks: 802643
Tim I move you from QA contact to Assigned. I believe that was your first intent.
Assignee: nobody → ttaubert
QA Contact: ttaubert
Priority: -- → P3
Reassigning this to myself since I'm doing it anyway as part of my work in 796729
Assignee: ttaubert → dflanagan
I've attached a patch to bug 796729 that gets rid of most (all?) of the reopen calls.  See https://github.com/mozilla-b2g/gaia/pull/6231
David, would you update the patch please?
Created attachment 687163 [details]
alive's pull request

I'm reassigning this bug to Alive and attaching a link to his pull request, which he did for bug 816878.  Also granting review+ for that pull request, with the conditions below:

See the comments in the pull request.  There are a few console.log() statements to remove, and one test_app that needs the same fix applied.

The code looks good to me, but please add a comment describing how you tested the affected activities before you land this patch.
Attachment #687163 - Flags: review+

Updated

6 years ago
Duplicate of this bug: 816878
Also, when you update your pull request, remember to change the commit message to reference this bug instead of 816878.

Thanks!
Hmm. I thought I reassigned this. Re-assining to Alive now.
Assignee: dflanagan → alive
(In reply to David Flanagan [:djf] from comment #6)
> The code looks good to me, but please add a comment describing how you
> tested the affected activities before you land this patch.

The test is easy to state.
Now any activity frame doesn't block home button, therefore we could try these 
1. Open dialer app, type some number, click add contact button
2. Open sms app, click + button, click contact icon to invoke contact app
3. Open e-mail app, click new mail, click + to pick a contact
4. Open UItest=>InlineActivity=>Click Test
5. Open UItest=>Contact test=>Click pick/new a contact

When the activity frame is up, click home button.
Homescreen should then shows and the app requests the activity should not be reopened.
Github is down now. I will land it after github resumes.

Thank you David Flanagan, I thought these workaround should be removed long time ago but I got some trouble in another bug and finally recognized that it's because reopenApp isn't removed :( So I filed that patch, I don't want others to waste their time on investigating who is launching the app...app itself.
Alive: just to be clear... Did you actually run the tests you describe above? In addition to testing that the Home button works as expected, please also verify that the activities still work correctly if you don't push the Home button!

Thanks for fixing this. I didn't realize that there were any possible side effects to the reopen code, or I would have tried harder to get my own patch landed. Sorry!
(In reply to David Flanagan [:djf] from comment #12)
> Alive: just to be clear... Did you actually run the tests you describe
> above? In addition to testing that the Home button works as expected, please
> also verify that the activities still work correctly if you don't push the
> Home button!
> 

Oh, I forgot to describe this part!

This afternoon, in order to make sure all things are well done,
I first time used the facebook import utility to increase my contacts. Because I didn't have any.
I think all these apps are related to contact picking/creating so this is the fastest way to do real test.
And I use the email to pick one of my ex-colleague who exposes his email on facebook. But sign, this doesn't work because another bug about facebook import. So I created one contact with an email to test. I created with my name and my mozilla mail account. It works because the gmail did send the mail to my mozilla mailbox.

I did the same thing to sms app to send a message to my own phone, and I did get the message.

The remaining apps are not related to data passing so I don't have something to do, just click around to see if something bad happened.

BTW, I only have 100+ contacts in my facebook account, but after I imported, the contact app got very very bad performance....

> Thanks for fixing this. I didn't realize that there were any possible side
> effects to the reopen code, or I would have tried harder to get my own patch
> landed. Sorry!

It's not your fault ;)
Additionally, the modification in system app is to remove outdated codes. Confirmed with timdream.
I also tested the ones in ui-test app just now.

Github seems recovered but I cannot press the merge button even it's green..sigh, I bet there's still something wrong in github so I had better not to merge before it goes stable.

Comment 15

6 years ago
What's the user impact here?
Target Milestone: --- → B2G C2 (20nov-10dec)
(In reply to Chris Lee [:clee] from comment #15)
> What's the user impact here?

Wrong application window popping up unsuspectingly when you press the home button.
https://github.com/alivedise/gaia/commit/756181bd1bfbce1b86e56a96cbb7b0ec1ce66a4b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 18

6 years ago
Verified on Unagi device Build ID: 20130114073222 v 1.0.0-Prerelease
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.