Add telemetry to measure whether user has no default browser set

RESOLVED WONTFIX

Status

()

Firefox for Android
General
RESOLVED WONTFIX
3 years ago
3 years ago

People

(Reporter: Margaret, Assigned: Margaret)

Tracking

Trunk
All
Android
Points:
---

Firefox Tracking Flags

(fennec33+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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

3 years ago
Assignee: nobody → margaret.leibovic
tracking-fennec: --- → ?
(Assignee)

Comment 1

3 years ago
Created attachment 8486562 [details] [diff] [review]
WIP - Add telemetry to measure whether user has no default browser set

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)
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.
> 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)
If we change it, just make sure it works. The current code successfully handles the the question of "is fennec the default?".
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

3 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

3 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

3 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)
(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)
(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.
tracking-fennec: ? → 33+
(Assignee)

Comment 11

3 years ago
Created attachment 8488545 [details] [diff] [review]
Debugging WIP v2 - Using queryIntentActivities

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

3 years ago
Attachment #8486562 - Attachment is obsolete: true
Sorry, Margaret, I haven't got around to poking at this yet. Tomorrow!
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

3 years ago
Attachment #8488545 - Flags: feedback?(rnewman)
(Assignee)

Comment 14

3 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
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.