Closed Bug 1063193 Opened 10 years ago Closed 10 years ago

Performing searches with Wikipedia search engine immediately launches Fennec

Categories

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

All
Android
defect

Tracking

(firefox35 verified, firefox36 verified, firefox37 verified, fennec35+)

VERIFIED FIXED
Firefox 35
Tracking Status
firefox35 --- verified
firefox36 --- verified
firefox37 --- verified
fennec 35+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

(Keywords: reproducible)

Attachments

(1 file, 1 obsolete file)

This is similar to the problem I was having for facets in bug 1062389.

We have logic in place that launches Fennec if the host of the URL we're trying to load doesn't match the host of the original search URL, but sometimes there can be redirects to other sub-domains.

I feel like we need a better way to determine whether or not to open Fennec, but I'm not sure what that should be.
Keywords: reproducible
I get this in different locales too such as ru_RU and fr_FR I just tested. All searches just launch the browser.
tracking-fennec: --- → ?
Assignee: nobody → margaret.leibovic
tracking-fennec: ? → 35+
So, the search plugin specifies "https://en.wikipedia.org/wiki/Special:Search" as the template, but it redirects to "https://en.m.wikipedia.org/wiki/Special:Search".

Fixing the template would fix this issue (and it would avoid a redirect). mconnor, can we do that? This change would affect Fennec as well, but I think it would be a positive change.

However, interesting side effect of the way wikipedia searches work is that you'll probably never leave the search activity, since the results aren't a list of external websites. In fact, usually the results just redirect to an article. If you really love wikipedia, this seems like a good way to get fast results, but I'm not sure how it fits into Fennec's story. Maybe it will fit in better if we ever allow you to simultaneously search from multiple engines (or let you switch engines more easily).
Flags: needinfo?(mconnor)
I think Wikipedia can UA sniff to switch between Phone and Tablet. If we switch the URL, let's make sure to add a tablet fallback, like we do in other XML files.
(In reply to Mark Finkle (:mfinkle) from comment #3)
> I think Wikipedia can UA sniff to switch between Phone and Tablet. If we
> switch the URL, let's make sure to add a tablet fallback, like we do in
> other XML files.

Testing on my N7, Wikipedia searches are still redirecting to en.m.wikipedia.com. I can add in a tablet <Url> tag like we do for Bing in case this changes, but it doesn't look like that would make a difference now.
Can I just do this? As I mentioned in my last comment, wikipedia doesn't serve a desktop site to tablets (also tested on a 10 inch tablet), so this will save us a redirect in Fennec as well.
Attachment #8496244 - Flags: review?(mconnor)
Attachment #8496244 - Flags: review?(mark.finkle)
If they're redirecting everything, this is the right fix, but we should probably tell them that's... lame.
Flags: needinfo?(mconnor)
Comment on attachment 8496244 [details] [diff] [review]
Update Wikipedia search plugin so that it won't redirect to a new domain

Right. Let's save a redirect for now. We can reach out to Wikipedia for a fix on their end so we get a better UX on 10" tablets.
Attachment #8496244 - Flags: review?(mark.finkle) → review+
Attachment #8496244 - Flags: review?(mconnor)
https://hg.mozilla.org/integration/fx-team/rev/2b67ef67b6a9

flod, will localizers see this change? This is something we would probably want to update in localized search plugins as well.
Flags: needinfo?(francesco.lodolo)
They'll see it, but they're also not authorized to make changes to searchplugins without review (unless we whitelist the change) :-\

And the worst part is that I finished fixing those URLs for bug 825821 not so long ago.
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #9)
> They'll see it, but they're also not authorized to make changes to
> searchplugins without review (unless we whitelist the change) :-\
> 
> And the worst part is that I finished fixing those URLs for bug 825821 not
> so long ago.

Oh, I didn't know about that bug. But mfinkle should have!

https://bugzilla.mozilla.org/show_bug.cgi?id=825821#c4

This makes me question the patch we went with in this bug. Maybe we should add the tablet specific URL, and allow that do to the redirect.
(In reply to :Margaret Leibovic from comment #10)

> > And the worst part is that I finished fixing those URLs for bug 825821 not
> > so long ago.
> 
> Oh, I didn't know about that bug. But mfinkle should have!
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=825821#c4
> 
> This makes me question the patch we went with in this bug. Maybe we should
> add the tablet specific URL, and allow that do to the redirect.

Zoinks! I'm shaking my fist at "Request Desktop Site". I'd be happy to r+ the phone/tablet split patch. Just make sure we fallback in a way that makes all 3 cases (phone/tablet/request-desktop-site) work.
If I go to the en.m.wikipedia.org URL with a desktop UA, I get redirected to the desktop site.  Same with Request Desktop Site.  I think the current patch is fine.

On the l10n issue, I wonder how hard it'd be to get Wikipedia to set up cnames so "{moz:locale}.m.wikipedia.org" Just Works?

Also, Wikipedia has greenlit using SSL, so we should be changing all of the links anyway...
(In reply to Mike Connor [:mconnor] from comment #12)
> Also, Wikipedia has greenlit using SSL, so we should be changing all of the
> links anyway...

That started a couple of weeks ago (bug 1069123).

To be honest I'm not sure how to reproduce this bug based on comment 0, but someone should check with the next en-US build if it actually breaks bug 825821 again.
https://hg.mozilla.org/mozilla-central/rev/2b67ef67b6a9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
(In reply to Francesco Lodolo [:flod] from comment #13)
> (In reply to Mike Connor [:mconnor] from comment #12)
> > Also, Wikipedia has greenlit using SSL, so we should be changing all of the
> > links anyway...
> 
> That started a couple of weeks ago (bug 1069123).
> 
> To be honest I'm not sure how to reproduce this bug based on comment 0, but
> someone should check with the next en-US build if it actually breaks bug
> 825821 again.

Sigh, the change here does break bug 825821 again. I think I should just back this out.

However, that would mean we need to something else to solve our search activity issue. Maybe we should add a new <Url> form just to be used in the search activity? Or I can hard-code a fix in the search activity to ignore wikipedia re-directs, but that doesn't feel great. However, the nice part of that would be that we wouldn't need to update search plugins in all the locales.

mfinkle, what do you think?
Flags: needinfo?(mark.finkle)
(In reply to :Margaret Leibovic from comment #15)

> Sigh, the change here does break bug 825821 again. I think I should just
> back this out.
> 
> However, that would mean we need to something else to solve our search
> activity issue. Maybe we should add a new <Url> form just to be used in the
> search activity? Or I can hard-code a fix in the search activity to ignore
> wikipedia re-directs, but that doesn't feel great. However, the nice part of
> that would be that we wouldn't need to update search plugins in all the
> locales.

So many great choices from which to pick! I wouldn't mind experimenting with a new <Url> just to see if it would work. Maybe look at the "rel" type info that OpenSearch supports?

We can always do the easier/hackier hard coded fix.
Flags: needinfo?(mark.finkle)
Verified as fixed in build 35.0a1 (2014-10-02);
Device Nexus 7 (Android 4.4.4);
Status: RESOLVED → VERIFIED
Backed out the first approach:
https://hg.mozilla.org/integration/fx-team/rev/39010225dc9f

I'll see what I can do with a new <Url>. Right now the parser just sets any type="text/html" as the results URI:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/providers/SearchEngine.java#126

I'll look at the OpenSearch spec, but we could also do something hacky like type="application/x-moz-searchacvitiy".
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Let's do it as a rel value please.  I'd like to kill the type hack (yeah, I need to file that bug) when the world is less insane.
Attachment #8496244 - Attachment is obsolete: true
This fixes the issue by adding a new <Url rel="mobile"> to the Wikipedia plugin.

I wasn't really sure what to use as the rel value for this, but I chose "mobile" since that's what the URL actually reflects.

We currently don't really handle multiple URLs in the search plugin parser, so I just added some basic logic to handle parsing more than one URL, and to use this "mobile" URL if it exists.
Attachment #8501424 - Flags: review?(bnicholson)
Attachment #8501424 - Flags: a11y-review?(mark.finkle)
Attachment #8501424 - Flags: a11y-review?(mark.finkle) → feedback?(mark.finkle)
Comment on attachment 8501424 [details] [diff] [review]
Add mobile-specific URL to wikipedia search plugin

I think this is a good progression to move more of the engines over to "rel" URLs. I suppose we could also create a <Url rel="tablet"> someday, when we need it.

MConnor might be pleased
Attachment #8501424 - Flags: feedback?(mark.finkle) → feedback+
Comment on attachment 8501424 [details] [diff] [review]
Add mobile-specific URL to wikipedia search plugin

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

Blegh, I can't say I like this since we're just building hacks on top of hacks. The fundamental problem here is relying on the domain to detect when we leave the search results page in the first place, which is fragile. For instance, search "analytics" with the Google search provider, and you don't leave the search activity when clicking the first result. But until we have providers giving us an API to build this thing the right way, we're kind of stuck.

Another option would be to have a custom set of "redirect rules", which doesn't sound too terrible to me since we already have the site-specific CSS rules. That would have the benefit of not needing to modify our search engine files specifically for the search activity, and would give us the ability to weed out edge cases like the example above.
Attachment #8501424 - Flags: review?(bnicholson) → review+
(In reply to Brian Nicholson (:bnicholson) from comment #22)
> Comment on attachment 8501424 [details] [diff] [review]
> Add mobile-specific URL to wikipedia search plugin
> 
> Review of attachment 8501424 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Blegh, I can't say I like this since we're just building hacks on top of
> hacks. The fundamental problem here is relying on the domain to detect when
> we leave the search results page in the first place, which is fragile. For
> instance, search "analytics" with the Google search provider, and you don't
> leave the search activity when clicking the first result. But until we have
> providers giving us an API to build this thing the right way, we're kind of
> stuck.
> 
> Another option would be to have a custom set of "redirect rules", which
> doesn't sound too terrible to me since we already have the site-specific CSS
> rules. That would have the benefit of not needing to modify our search
> engine files specifically for the search activity, and would give us the
> ability to weed out edge cases like the example above.

Yeah... our current system is very much less than ideal :/

One thing that would be nice about the redirect rule is that we wouldn't need to update the localized plugins. But any real robust fix would likely involve updating the plugins anyway.

Now I'm having mixed feelings about this approach... maybe we should just try adding some additional logic in the isSearchResultsPage method.
Blocks: 1081136
I decided to just land what we have here, and I filed bug 1081136 about trying to create a more robust solution in the future. However, that bug may require working with search providers to actually know whether or not a page is a "results" page.

https://hg.mozilla.org/integration/fx-team/rev/320dd3495026
https://hg.mozilla.org/mozilla-central/rev/320dd3495026
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Performing searches with Wikipedia search engine doesn't launch Fennec. Verified as fixed in latest builds:
Firefox for Android 37.0a1 (2014-12-09)
Firefox for Android 36.0a2 (2014-12-09)
Firefox for Android 35.b1

Device: Nexus 4 (Android 4.4.4)
Status: RESOLVED → VERIFIED
Blocks: 1126700
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.