Firefox doesn't integrate with Android's Web Search "Intent"

ASSIGNED
Assigned to

Status

()

Firefox for Android
Search Activity
P3
normal
ASSIGNED
8 months ago
2 months ago

People

(Reporter: u583025, Assigned: maliu)

Tracking

({leave-open})

50 Branch
Unspecified
Android
leave-open
Points:
---

Firefox Tracking Flags

(firefox53blocking wontfix, firefox54 wontfix, firefox55 affected)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

8 months ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161114144856

Steps to reproduce:

Android has something called a Web Search "Intent" - https://developer.android.com/reference/android/content/Intent.html#ACTION_WEB_SEARCH
It can take keywords from apps (like text from an ebook reader say) and searches them in any app that follows the Web Search "Intent".


I have an ebook reader app which has implemented the web search intent to be passed on to apps which support it, like Google's own Search app. Moreover, Bing, Opera Browser, Dolphin Browser and others support it too. Firefox doesn't

Here is the GIF showcasing the same - http://imgur.com/9NAYQwK

Now considering that Firefox already has a Search Activity, which acts as Assists and acts separate from the browser, it would be great if the Search Activity followed the Intent and gave the desired search results. 


Actual results:

Firefox should have read the Android Web Search "Intent" and be displayed among the options for when an Intent is asked for.


Expected results:

Firefox/ Firefox Search Activity don't show up as a  browser/search app that can read the Android Web Search Intent.
(Reporter)

Updated

8 months ago
OS: Unspecified → Android
See Also: → bug 1256240
Shouldn't be too hard. Flagging for triage: Maybe a front-end developer has time to implement this. This could potentially improve engagement a bit.
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
Priority: -- → P3
(Reporter)

Updated

8 months ago
Priority: P3 → P5
(Reporter)

Comment 2

8 months ago
Considering that all famous browsers and search apps use search "intent" since a long time and many 3rd party apps have integrated the ability to launch the intent, I think it would improve Firefox quite noticeably.
Actually... this is something for the roadmap (and not triage). Barbara, what do you think?
tracking-fennec: ? → ---
Flags: needinfo?(bbermes)
Priority: P5 → P3
(Assignee)

Updated

8 months ago
Assignee: nobody → max
Definitely something to put on the roadmap for "Intent" work. I see other Intent plans for more important for now, unless it's easy to do (work-wise/effort-wise)?

Please include probes to understand the usage of this intent.
Flags: needinfo?(bbermes) → needinfo?(s.kaspari)
Yeah, compared with other Intent based features this should be pretty easy.
Flags: needinfo?(s.kaspari)
(Reporter)

Comment 6

8 months ago
Any progress on this?
Comment hidden (mozreview-request)

Comment 8

7 months ago
mozreview-review
Comment on attachment 8818831 [details]
Bug 1320072 - Support direct search from intent,

https://reviewboard.mozilla.org/r/98772/#review99378

LGTM. I'm just not sure about the state when the activity is getting restored.

::: mobile/android/search/java/org/mozilla/search/SearchActivity.java:180
(Diff revision 1)
> -        if (savedInstanceState != null) {
> +        final String queryString = getQueryString();
> +        if (!TextUtils.isEmpty(queryString)) {
> +            searchBar.setText(queryString);
> +            onSearch(queryString);
> +        } else if (savedInstanceState != null) {

I wonder if the block checking savedInstanceState should be before this one?

What if we start the activity with a search intent, perform some other searches, leave the activity and later return (activity gets restored): Will be run the initial query again instead of restoring the last query?
Attachment #8818831 - Flags: review?(s.kaspari) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 10

7 months ago
(In reply to Sebastian Kaspari (:sebastian) from comment #8)
> Comment on attachment 8818831 [details]
> ::: mobile/android/search/java/org/mozilla/search/SearchActivity.java:180
> (Diff revision 1)
> > -        if (savedInstanceState != null) {
> > +        final String queryString = getQueryString();
> > +        if (!TextUtils.isEmpty(queryString)) {
> > +            searchBar.setText(queryString);
> > +            onSearch(queryString);
> > +        } else if (savedInstanceState != null) {
> 
> I wonder if the block checking savedInstanceState should be before this one?
> 
> What if we start the activity with a search intent, perform some other
> searches, leave the activity and later return (activity gets restored): Will
> be run the initial query again instead of restoring the last query?

I guess you are correct, but there could also be another problem. If we always take savedInstanceState prior than incoming query, then new query is not been handled as long as savedInstanceState exist. So I made some modification on the patch. Would you help to take a look please?
Flags: needinfo?(s.kaspari)
Comment on attachment 8818831 [details]
Bug 1320072 - Support direct search from intent,

Sure! Transforming the NI into a review request.
Flags: needinfo?(s.kaspari)
Attachment #8818831 - Flags: review+ → review?(s.kaspari)

Comment 12

6 months ago
mozreview-review
Comment on attachment 8818831 [details]
Bug 1320072 - Support direct search from intent,

https://reviewboard.mozilla.org/r/98772/#review105288

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/SafeIntent.java:79
(Diff revision 2)
>              Log.w(LOGTAG, "Couldn't get intent extras.", e);
>              return null;
>          }
>      }
>  
> +    public Bundle getExtras() {

nit: This is not the convention in this file yet, but I'd either return an empty Bundle in the exception case or add @Nullable to the method signature.
Attachment #8818831 - Flags: review?(s.kaspari) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

6 months ago
Keywords: checkin-needed

Comment 14

6 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d24c7655825c
Support direct search from intent, r=sebastian
Keywords: checkin-needed

Comment 15

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d24c7655825c
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53

Comment 16

3 months ago
I don't believe that PROCESS_TEXT should have been added here. 

See bug 1360670.

Comment 17

3 months ago
We are going to need to back out this change for a Firefox 53.0.2.

The changes it made to Android integration are affecting our preloads.

We need to spend more time investigating these integration points.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 18

3 months ago
[Tracking Requested - why for this release]: Our partners are having issues with these changes on their preloads because of how it changes our Android integration.
tracking-firefox53: --- → ?

Comment 19

3 months ago
I've verified that this patch backs out cleanly (no conflicts).
Should we back this out from other branches? We can respin a new Fennec quickly, but I'd like to make sure we know we're doing the right thing. 

Are we likely to break something else from the backout?

How can we handle QA with/for distribution partners in a better way?
From more discussion on irc with Sebastian and Mike, Mike will get backout patch ready for all channels.
tracking-firefox53: ? → blocking
Comment hidden (mozreview-request)

Comment 23

3 months ago
The backout patch applies cleanly to release and beta. I'm putting together a separate patch for inbound that I will check in.

Comment 24

3 months ago
I was wrong about inbound. I had a local patch. Once reviewed, I'll check in there.

Comment 25

3 months ago
Comment on attachment 8863460 [details]
Bug 1320072 - Backout intent change - broke partner Google test.

This is a backout, it shouldn't need review in hindsight.

Approval Request Comment
[Feature/Bug causing the regression]: This bug (1320072)
[User impact if declined]: Firefox won't ship on new devices
[Is this code covered by automated tests?]: No (it wasn't before either)
[Has the fix been verified in Nightly?]: Hasn't been backed out there yet.
[Needs manual test from QE? If yes, steps to reproduce]: Verify that Firefox is not in the Web Search when long pressing text in other apps.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Backs out new code.
[String changes made/needed]: None
Attachment #8863460 - Flags: approval-mozilla-release?
Attachment #8863460 - Flags: approval-mozilla-beta?

Comment 26

3 months ago
Comment on attachment 8863460 [details]
Bug 1320072 - Backout intent change - broke partner Google test.

via IRC
Attachment #8863460 - Flags: review?(snorp) → review+
Did the backout land on release? The flags were set to "fixed" so the sheriffs wouldn't see this.
Flags: needinfo?(mozilla)
Comment on attachment 8863460 [details]
Bug 1320072 - Backout intent change - broke partner Google test.

Land this for a dot release
Attachment #8863460 - Flags: approval-mozilla-release?
Attachment #8863460 - Flags: approval-mozilla-release+
Attachment #8863460 - Flags: approval-mozilla-beta?
Attachment #8863460 - Flags: approval-mozilla-beta+

Comment 29

3 months ago
Not sure what to do with the flags here now.

The original bug is not fixed.

I've backed this out of beta and release.
Flags: needinfo?(mozilla)

Comment 30

3 months ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7894c708b6f
Backout intent change - broke partner Google test. r=snorp
For posterity:
https://hg.mozilla.org/releases/mozilla-beta/rev/014fa3e151f8
https://hg.mozilla.org/releases/mozilla-release/rev/44981f78f602
Status: REOPENED → ASSIGNED
status-firefox53: fixed → wontfix
status-firefox54: --- → wontfix
status-firefox55: --- → affected
Target Milestone: Firefox 53 → ---
Keywords: leave-open

Updated

3 months ago
Duplicate of this bug: 1360670

Comment 33

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a7894c708b6f
(Reporter)

Comment 34

2 months ago
(In reply to Mike Kaply [:mkaply] from comment #29)
> Not sure what to do with the flags here now.
> 
> The original bug is not fixed.
> 
> I've backed this out of beta and release.

Yeah, I'd still like to have this feature if possible. Its very handy.

Comment 35

2 months ago
> Yeah, I'd still like to have this feature if possible. Its very handy.

Totally understand. We're trying to figure out how to make it happen.
You need to log in before you can comment on or make changes to this bug.