Closed
Bug 801520
Opened 13 years ago
Closed 13 years ago
Gaia part of bug 798445: Remove |activity.onsuccess = reopenApp| callbacks.
Categories
(Firefox OS Graveyard :: Gaia, defect, P3)
Firefox OS Graveyard
Gaia
Tracking
(blocking-basecamp:+)
People
(Reporter: timdream, Assigned: alive)
References
Details
Attachments
(1 file)
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.
Comment 2•13 years ago
|
||
Tim I move you from QA contact to Assigned. I believe that was your first intent.
Assignee: nobody → ttaubert
QA Contact: ttaubert
Updated•13 years ago
|
Priority: -- → P3
Comment 3•13 years ago
|
||
Reassigning this to myself since I'm doing it anyway as part of my work in 796729
Assignee: ttaubert → dflanagan
Comment 4•13 years ago
|
||
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
Reporter | ||
Comment 5•13 years ago
|
||
David, would you update the patch please?
Comment 6•13 years ago
|
||
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+
Comment 8•13 years ago
|
||
Also, when you update your pull request, remember to change the commit message to reference this bug instead of 816878.
Thanks!
Comment 9•13 years ago
|
||
Hmm. I thought I reassigned this. Re-assining to Alive now.
Assignee: dflanagan → alive
Assignee | ||
Comment 10•13 years ago
|
||
(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.
Assignee | ||
Comment 11•13 years ago
|
||
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.
Comment 12•13 years ago
|
||
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!
Assignee | ||
Comment 13•13 years ago
|
||
(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 ;)
Assignee | ||
Comment 14•13 years ago
|
||
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 16•13 years ago
|
||
(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.
Assignee | ||
Comment 17•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 18•12 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.
Description
•