Closed
Bug 1243931
Opened 9 years ago
Closed 9 years ago
Search activity doesn't work when used from a library
Categories
(Firefox for Android Graveyard :: Search Activity, defect)
Firefox for Android Graveyard
Search Activity
Tracking
(firefox47 fixed)
RESOLVED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: toonetown, Assigned: toonetown)
Details
Attachments
(1 file)
4.20 KB,
patch
|
nalexander
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → nathan
Attachment #8713390 -
Flags: review?(nalexander)
Comment 2•9 years ago
|
||
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.
Attachment #8713390 -
Flags: review?(nalexander) → review+
Assignee | ||
Comment 3•9 years ago
|
||
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•9 years ago
|
||
(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.
Status: NEW → ASSIGNED
Keywords: checkin-needed
Keywords: checkin-needed
Comment 6•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Updated•7 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
•