Closed Bug 899141 Opened 6 years ago Closed 6 years ago

URLs are not opened from other apps if the "Don't keep activities" option is set

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 26
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- verified
firefox26 --- verified
fennec 23+ ---

People

(Reporter: AdrianT, Assigned: bnicholson)

Details

Attachments

(1 file, 1 obsolete file)

Nightly 25.0a1 2013-07-28
Asus EEE Transformer TF101 (Android 4.0.4)

Steps to reproduce:
1) Set the "Don't keep activities" developer option to ON
2) Open Firefox and load any website
3) Minimize the app
4) Open a link from another app (Twitter, Gmail app) using Firefox

Expected result:
The website is loaded in a new tab

Actual result:
No new tabs are created and the website is never loaded
If we're already running, we skip over the call to loadStartupTab(), but we shouldn't do that if we're given a new URL to load.
Assignee: nobody → bnicholson
Status: NEW → ASSIGNED
Attachment #784425 - Flags: review?(mark.finkle)
tracking-fennec: ? → 23+
Comment on attachment 784425 [details] [diff] [review]
Always open a startup tab if given a URL

Add a comment above the "if" block:
// External URLs should always be loaded regardless of how we are restoring

(or something like that)
Attachment #784425 - Flags: review?(mark.finkle) → review+
Comment on attachment 784425 [details] [diff] [review]
Always open a startup tab if given a URL

Review of attachment 784425 [details] [diff] [review]:
-----------------------------------------------------------------

Thinking about this more, this will still lead to problems. Consider the following STR:

1) Launch Fennec with an intent to open some URL
2) Cause Fennec to be killed in the background
3) Reopen Fennec from the recent apps list

Fennec will be reopened in step 3 without a new intent, meaning it will still use the intent from step 1. And since the activity gets recreated, this code path gets hit again. Note that the proper fix here, whatever it will be, may overlap with bug 896170.
Attachment #784425 - Flags: review+
I think we can simplify our intent handled logic by simply checking for the existence of savedInstanceState. savedInstanceState will be non-null after the activity is killed (either with DKA or Fennec being background killed), and in those cases, onCreate() will always be called with the prior intent (and the new intent is given to onNewIntent(), which is called immediately after).

I removed some old code from bug 732268 that checked if we were being launched from the recent apps list. We don't need this anymore since we're clearing the old intent in onCreate(). I also removed our ACTION_MAIN handling since that only creates an empty URI load event, which doesn't actually do anything.

Mark suggested adding some tests for these, which I think is a good idea. Though we probably won't be able to enable Don't Keep Activities directly in robocop, we might be able to add special code in onDestroy() to prevent killing Gecko when the activity gets destroyed. That would allow us to create the activity again, essentially reproducing DKA's behavior.

Also including Kats for review since he reviewed bug 896992.
Attachment #784425 - Attachment is obsolete: true
Attachment #784739 - Flags: review?(mark.finkle)
Attachment #784739 - Flags: review?(bugmail.mozilla)
(In reply to Brian Nicholson (:bnicholson) from comment #4)
> Mark suggested adding some tests for these, which I think is a good idea.

Filed bug 900799.
Comment on attachment 784739 [details] [diff] [review]
Fix startup intent handling

The approach seems like it should work and is cleaner.

I assume you have tested the cases where you are removing code to make sure the bugs fixed by that code are still fixed.
Attachment #784739 - Flags: review?(mark.finkle) → review+
Comment on attachment 784739 [details] [diff] [review]
Fix startup intent handling

Review of attachment 784739 [details] [diff] [review]:
-----------------------------------------------------------------

This is definitely much nicer, if it works! I'm a little concerned that this might break existing codepaths that we rely on to restore tabs when going back into Fennec, but I guess if session restore handles all of those cases then it should be fine. It should at least be simpler to update the session restore code to restore things if this does break stuff since the logic is simpler now. r=me but this will need a good amount of testing. I'll be sure to run with DKA once this lands to test as well.
Attachment #784739 - Flags: review?(bugmail.mozilla) → review+
Also CC'ing liuche as FYI, since this basically removes all the code she added recently.
https://hg.mozilla.org/mozilla-central/rev/de0b096994cf
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Duplicate of this bug: 896170
Verified as fixed on:
Device: LG Nexus 4 (Android 4.2.2)
Build: Nightly 26.0a1 (2013-08-15)
Comment on attachment 784739 [details] [diff] [review]
Fix startup intent handling

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unknown
User impact if declined: tabs may not load if don't keep activities is enabled
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk; baked on m-c for several weeks
String or IDL/UUID changes made by this patch: none
Attachment #784739 - Flags: approval-mozilla-aurora?
Attachment #784739 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on:
Build: Firefox for Android 25.0b1 (2013-09-18)
Device: LG Nexus 4 
OS: Android 4.2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.