Closed
Bug 1064857
Opened 10 years ago
Closed 10 years ago
Add telemetry to measure whether user has no default browser set
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec33+)
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
fennec | 33+ | --- |
People
(Reporter: Margaret, Assigned: Margaret)
References
Details
Attachments
(1 file, 1 obsolete file)
5.35 KB,
patch
|
Details | Diff | Splinter Review |
Bug 1030935 added a probe to measure whether or not the browser is the default, but it would also be useful to know whether or not the user just has *no* default browser set, since we've been speculating that users who don't have Fennec set as a default might just like choosing their browser every time.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → margaret.leibovic
tracking-fennec: --- → ?
Assignee | ||
Comment 1•10 years ago
|
||
This is what I imagined this patch should look like, but when I tested locally with some logging, I found that when I reset my defaults, there was still a default, but the package name for that was "android".
Should we add a check for this and treat it like "no default"?
Attachment #8486562 -
Flags: feedback?(mark.finkle)
Comment 2•10 years ago
|
||
resolveActivity:
"Returns a ResolveInfo containing the final activity intent that was determined to be the best action. Returns null if no matching activity was found. If multiple matching activities are found and there is no default set, returns a ResolveInfo containing something else, such as the activity resolver."
We should probably be using
http://developer.android.com/reference/android/content/pm/PackageManager.html#queryIntentActivities%28android.content.Intent,%20int%29
If the returned list is empty, there are no apps suitable. (Should not occur unless we specify ourselves as the caller.)
If the first returned element has `isDefault = true`, then there's a default app set.
Otherwise, the user hasn't specified one.
Comment 3•10 years ago
|
||
> If the first returned element has `isDefault = true`, then there's a default
> app set.
Wait, that's not true. That's still about manifest entries.
We probably need to check for priority numbers. If the first item has priority 0, it's non-default. Either that or ordering.
(See also http://developer.android.com/reference/android/content/IntentFilter.html#MATCH_CATEGORY_MASK)
Comment 4•10 years ago
|
||
If we change it, just make sure it works. The current code successfully handles the the question of "is fennec the default?".
Comment 5•10 years ago
|
||
Comment on attachment 8486562 [details] [diff] [review]
WIP - Add telemetry to measure whether user has no default browser set
Isn't this patch a duplicate of what we currently do? Oh wait. FENNEC_IS_USER_DEFAULT_SET is throwing me for a loop.
I think it would be more understandable with BROWSER_IS_ANY_DEFAULT_SET.
The only other thing to consider is whether we'd want to save this data whenever the method is called, or only in response to "Telemetry:Gather" as we already do.
If we move this to the "Telemetry:Gather" code path (I kinda think we should) then this method should probably return an Enum.
Attachment #8486562 -
Flags: feedback?(mark.finkle) → feedback-
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #5)
> Comment on attachment 8486562 [details] [diff] [review]
> WIP - Add telemetry to measure whether user has no default browser set
>
> Isn't this patch a duplicate of what we currently do? Oh wait.
> FENNEC_IS_USER_DEFAULT_SET is throwing me for a loop.
>
> I think it would be more understandable with BROWSER_IS_ANY_DEFAULT_SET.
I went with the FENNEC prefix because this is fennec-only. In fact the description for BROWSER_IS_USER_DEFAULT should be updated because it says it's just for desktop.
But I agree _IS_ANY_DEFAULT_SET is more clear that IS_USER_DEFAULT_SET.
> The only other thing to consider is whether we'd want to save this data
> whenever the method is called, or only in response to "Telemetry:Gather" as
> we already do.
isDefaultBrowser() is only called when we gather telemetry, but it is more future-proof to put the probe in the "Telemetry:Gather" handler rather than in this helper method.
> If we move this to the "Telemetry:Gather" code path (I kinda think we
> should) then this method should probably return an Enum.
Sure, an enum sounds like the right solution for this.
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #3)
> > If the first returned element has `isDefault = true`, then there's a default
> > app set.
>
> Wait, that's not true. That's still about manifest entries.
>
> We probably need to check for priority numbers. If the first item has
> priority 0, it's non-default. Either that or ordering.
Whether I had a default set or not, they were always in the same order and they were all always priority 0. I also found that isDefault was always false for all of them. I'll keep playing around with different options...
Assignee | ||
Comment 8•10 years ago
|
||
Unless you have any good idea, I think checking if the default package name is "android" is the way forward here. However, that feels kinda hacky, so I would love for a better idea.
Flags: needinfo?(rnewman)
Comment 9•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #7)
> Whether I had a default set or not, they were always in the same order and
> they were all always priority 0. I also found that isDefault was always
> false for all of them. I'll keep playing around with different options...
Strange! Could you post code/patch? This should be the same way we compute helper apps, so it ought to work.
Flags: needinfo?(rnewman)
Comment 10•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #8)
> Unless you have any good idea, I think checking if the default package name
> is "android" is the way forward here. However, that feels kinda hacky, so I
> would love for a better idea.
To rephrase/restate, make sure I'm understanding: you're hoping to ask for the single best handler for a URL, using a package-based heuristic to determine if that handler is the activity chooser, rather than a browser, and assuming that otherwise the returned handler must be the default application?
My feeling is that that's a little risky, which is why I suggested the approach that doesn't include the activity chooser.
We don't know what the chooser package will be on a Mi3, Kindle, etc.
It's also possible that the default browser will be in that package if it's "Internet".
And it's also theoretically possible that Android will return a real activity (not the chooser) even with no default specified.
There are some open questions, too:
* When there's only one browser that matches -- say the user has disabled "Internet" in App Settings, or a pre-loaded Firefox -- do we count that as default? If we do, is it the same kind of "default" as being the default when multiple browsers are installed (a user choice)?
* When multiple browsers match, so there's no default, but they're all Firefoxes, do we care?
If we can make it work, the list-based approach would avoid or clarify most of these issues, so I think it's worth spending a little time on it. If we can't make it work, then yeah, I'd opt for checking the package versus checking the activity name itself.
Updated•10 years ago
|
tracking-fennec: ? → 33+
Assignee | ||
Comment 11•10 years ago
|
||
Here's the patch I was using to debug the list-based approach.
I totally agree with your last comment that an approach like this would be much less fragile than trying to figure out if the returned activity is the chooser package.
However, with this patch, I'm just seeing this in the log, regardless of whether or not I've set a default:
I/MMM (31953): name: com.android.chrome, priority: 0, isDefault: false
I/MMM (31953): name: org.mozilla.firefox, priority: 0, isDefault: false
I/MMM (31953): name: org.mozilla.firefox_beta, priority: 0, isDefault: false
I/MMM (31953): name: org.mozilla.fennec, priority: 0, isDefault: false
I/MMM (31953): name: org.mozilla.fennec_aurora, priority: 0, isDefault: false
I/MMM (31953): name: org.mozilla.fennec_leibovic, priority: 0, isDefault: false
Attachment #8488545 -
Flags: feedback?(rnewman)
Assignee | ||
Updated•10 years ago
|
Attachment #8486562 -
Attachment is obsolete: true
Comment 12•10 years ago
|
||
Sorry, Margaret, I haven't got around to poking at this yet. Tomorrow!
Comment 13•10 years ago
|
||
It turns out that queryIntentActivities simply doesn't do what it says it's supposed to do -- the results aren't ordered in any meaningful way.
getPreferredActivities *does* do what it's supposed to do. Apart from this horrendous typo bug on KitKat.
https://code.google.com/p/android/issues/detail?id=63245
This thread, from the horse's mouth:
https://groups.google.com/forum/#!topic/android-developers/UkfP70MtjGA
is terribly depressing, but both brief and accurate.
As such, I think there are three ways forward. Thanks to Alanis Morrisette for the inspiration for this bug comment.
The complex: do what Hackborn says, and use resolveIntentActivity, using a heuristic to detect the intent resolver. Tears will flow like rivers. This might or might not be exactly equivalent to your first patch.
The craziest: figure out how to call the method that Hackborn *says* is the right one, and I can see in the code:
http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/1.5_r4/com/android/server/PackageManagerService.java#1239
The sound of pretenses falling all around:
This code picks out Nightly six times from a list of 70 intents. It picks no other activity.
List<ComponentName> activities = new ArrayList<>();
List<IntentFilter> filters = new ArrayList<>();
getPackageManager().getPreferredActivities(filters , activities, null);
int i = 0;
for (IntentFilter intentFilter : filters) {
if (intentFilter.matchAction(Intent.ACTION_VIEW)) {
final int match = intentFilter.match(getContentResolver(), viewIntent, false, LOGTAG);
if (match > 0) {
Log.i("MMMRRR", "Match at " + i + " is " + match);
Log.i("MMMRRR", "Activity: " + activities.get(i).getPackageName());
}
}
i++;
Log.i("MMMRRR", "Intent filter: " + intentFilter);
}
But you know what? It doesn't tell you what the *current* default is. If I clear defaults, it prints the same.
So if we ever want to know if we *were* the default, this is how to do it.
Assignee | ||
Updated•10 years ago
|
Attachment #8488545 -
Flags: feedback?(rnewman)
Assignee | ||
Comment 14•10 years ago
|
||
This sounds way more complicated than I thought it would be, so I don't think it's worth it to pursue this farther. We were on the fence about whether or not we need this data, and I feel like bad data is worse than no data, so I'm gonna WONTFIX this.
If we really care about this, perhaps we can get some user research to study how users deal with multiple browsers on their devices, and why some users might not set a default.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•