1243931 - Search activity doesn't work when used from a library

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect)

Description

[Nathan Toone [:toonetown]]

Since the code in the search activity uses AppConstants.ANDROID_PACKAGE_NAME, if you happen to load that code from within a library, it doesn't work (because AppConstants.ANDROID_PACKAGE_NAME is baked in at compile time).

Suggestion on IRC (from nalexander) was to use the Android Context's getPackageName() function where it is available.

Comment 1

[Nathan Toone [:toonetown]]

Attachment: Replace AppConstants.ANDROID_PACKAGE_NAME with Context.getPackageName() in search widget

Comment 2

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8713390 [details] [diff] [review]
Replace AppConstants.ANDROID_PACKAGE_NAME with Context.getPackageName() in search widget

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

r+ with a nit and an open question.

::: mobile/android/search/java/org/mozilla/search/PostSearchFragment.java
@@ +133,5 @@
>                  final Intent i = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
>  
>                  // If the intent URI didn't specify a package, open this in Fennec.
>                  if (i.getPackage() == null) {
> +                    i.setClassName(view.getContext().getPackageName(), AppConstants.MOZ_ANDROID_BROWSER_INTENT_CLASS);

Google suggests lots of times when views in Fragments can have null context.  But you're in a Fragment -- is there a reason to expect `getActivity()` to be null?  No matter what, be defensive here: null-check the context and fallback to the hardcoded package name if things fail.

Comment 3

[Nathan Toone [:toonetown]]

It's actually not in a fragment - it's in a WebViewClient.  The function where this change is located is within the shouldOverrideUrlLoading callback of WebViewClient, and is passed a WebView.

Do you know if a WebView can ever have a null context?  I'm not able to find anything that would suggest it is.  This particular WebView would be created as part of the search_fragment_post_search.xml file.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Nathan Toone [:toonetown] from comment #3)
> It's actually not in a fragment - it's in a WebViewClient.  The function
> where this change is located is within the shouldOverrideUrlLoading callback
> of WebViewClient, and is passed a WebView.

Ta -- didn't see the complete context so I missed this.

> Do you know if a WebView can ever have a null context?  I'm not able to find
> anything that would suggest it is.  This particular WebView would be created
> as part of the search_fragment_post_search.xml file.

Let's assume the webView is not null.

Sheriff: this has no automated testing, so not pushing to try.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/2fabe64fbd82

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2fabe64fbd82