820576 - Search suggestion URLs should go directly to URL

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Brian Nicholson (:bnicholson)]

When typing "maps.g" and you click "maps.google.com", this should open http://maps.google.com directly rather than do a search for "maps.google.com".

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Part 1: Move isSearchUrl to StringUtils

Comment 2

[Brian Nicholson (:bnicholson)]

Attachment: Part 2: Open URL for suggestions that look like URLs

Comment 3

[Brian Nicholson (:bnicholson)]

Attachment: Part 2: Open URL for suggestions that look like URLs, v2

s/viewHolder.suggestionView/viewHolder.userEnteredView/

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 691071 [details] [diff] [review]
Part 1: Move isSearchUrl to StringUtils

Let's rename isSearchUrl -> isSearchQuery since a "search url" is kinda weird if you think about it.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 691079 [details] [diff] [review]
Part 2: Open URL for suggestions that look like URLs, v2


>                     if (listener != null) {
>                         String suggestion = ((TextView) v.findViewById(R.id.suggestion_text)).getText().toString();
>-                        listener.onSearch(engine.name, suggestion);
>+                        // If we're not clicking the user-entered view (the

Add a blank line before the comment

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

I just wanted to give Ian a chance to veto this feature. I have been wanting this for a while, but UX should be aware of the change.

Comment 7

[Ian Barlow (:ibarlow)]

Oh man. I 100% want this. We've talked about it within UX for some time, but I think it just never came up in other conversations. 

But yes. Please proceed!

Comment 8

[Wesley Johnston (:wesj)]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> Comment on attachment 691071 [details] [diff] [review]
> Part 1: Move isSearchUrl to StringUtils
> 
> Let's rename isSearchUrl -> isSearchQuery since a "search url" is kinda
> weird if you think about it.

If we're going to make this a string utility function, seems better to just ask "isUrl" since we may not be calling with a search term.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #8)
> (In reply to Mark Finkle (:mfinkle) from comment #4)
> > Comment on attachment 691071 [details] [diff] [review]
> > Part 1: Move isSearchUrl to StringUtils
> > 
> > Let's rename isSearchUrl -> isSearchQuery since a "search url" is kinda
> > weird if you think about it.
> 
> If we're going to make this a string utility function, seems better to just
> ask "isUrl" since we may not be calling with a search term.

I considered that too, but in the end I felt answering "isUrl" is much harder than "isSearchQuery" since URLs are tricky beasts and I didn't want "isUrl" to "mostly" work. Also, the code in the method was already designed to answer "isSearchQuery", so it required less work.

Comment 10

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e14b7800ae01
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ce57f5cca0b

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e14b7800ae01
https://hg.mozilla.org/mozilla-central/rev/1ce57f5cca0b

Comment 12

[Ioana Chiorean]

Firefox 20.0a1 (2012-12-17)
Device: Galaxy Nexus
OS: Android 4.1.1

URL Search suggestions go directly to correct URL (steps from Comment 1). Marking bug as VERIFIED FIXED.