1039766 - Pasting into url bar does not bring up search screen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

1) Copy something into your clipboard
2) Long press the url bar
3) Paste into the url bar

Expected: The awesomescreen is presented in the search state, the same state as if you typed some characters
Actual: The awesomescreen is presented on the home panels, preventing a quick search of the values.

Comment 1

[Aaron Train [:aaronmt]]

Regression?

Comment 2

[Teodora Vermesan (:TeoVermesan)]

I don't think this issue is a regression, because I have installed Firefox 26 and the awesomescreen is presented on the home panels.

Comment 3

[:Margaret Leibovic]

This seems like it would be a nice enhancement.

Comment 4

[Ashish Madeti [:madeti]]

I want to work on this bug but I'm a little confused. What I understand is, when we select "Paste" from the menu provided when we long press the url bar, we cannot do a quick search of the pasted contents. But if we paste while editing in the url bar, we can do a quick search.

What is this awesomescreen? (I tried to figure out from code, but no luck.) Have I understood the problem correctly? What files/code should I look into, to understand/solve it?

Comment 5

[Aaron Train [:aaronmt]]

Hi Ashish,

I changed mentor's as the former is away currently. The new mentor can help you out here.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, Ashish! Welcome to Bugzilla - you've been assigned.

(In reply to Ashish Madeti from comment #4)
> What is this awesomescreen? (I tried to figure out from code, but no luck.)
The awesomescreen is the general term we use to describe the state that allows you to navigate to new web content after the toolbar has been pressed (the equivalent feature on desktop is called the "awesomebar"). This state includes the home panels (which has history, bookmarks, top sites, etc.) and the search screen (that allows you to do quick searches, search through bookmarks/history via text, etc.). For whatever reason, the code is not named after this concept. 

> Have I understood the problem correctly?
Sounds correct to me!

> What files/code should I look into, to understand/solve it?
The context menu option can be found in BrowserApp.onContextItemSelected [1] and the search screen is defined by the BrowserSearch class [2].

Let me know if you have any further questions!

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java?rev=246fab00de4a#806
[2]; https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/BrowserSearch.java

Comment 7

[Ashish Madeti [:madeti]]

Attachment: A rough patch

Some issues (which I couldn't understand how to tackle):

1. If you paste in to the url bar (by long clicking), right after starting fennec, it will not work. That I guess, is because of the fact that mAutocompleteHandler of BrowserSearch has not been initialised. So this will work, if we first click the url bar, and enter something so that it shows quick search, and afterwards whenever we paste by long clicking, it will work. So how can I initialise mAutocompleteHandler, or what can I pass as AutocompleteHandler to filter().

2. For now I have just passed a null as AutocompleteHandler and am checking for that in filter(), should I rather make an overloaded filter() with just one argument (i.e searchTerm)

I hope I was able to explain you the problem

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8466745 [details] [diff] [review]
A rough patch

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

Thanks for the patch, Ashish!

Note that your bug description should describe what you fixed - not the issue itself. In this case, it should be something like, "Pasting into the url bar displays the search screen." You can update this by running `hg qrefresh -e` (note that this will also add all of the changes in your working directory, shown up `hg diff`, to the current patch, so be careful when you run it!).

(In reply to Ashish Madeti from comment #7)
> 1. If you paste in to the url bar (by long clicking), right after starting
> fennec, it will not work.

It works for me but I'm using a relatively quick device (Nexus 4). What device are you using? Do you have any special steps to reproduce?

> That I guess, is because of the fact that
> mAutocompleteHandler of BrowserSearch has not been initialised.

It seems that the AutocompleteHandler just handles finishing something a user typed in, e.g. "goo" might become "google.com". This behavior seems undesireable for this use case so I think we can procede without the AutocompleteHandler (i.e. pass in null). See my comments below on why mAutocompleteHandler may be null.

> 2. For now I have just passed a null as AutocompleteHandler and am checking
> for that in filter(), should I rather make an overloaded filter() with just
> one argument (i.e searchTerm)

It seems that other methods just pass in null - passing in null here should be fine (see my other comment below on why that is okay).

::: mobile/android/base/home/BrowserSearch.java
@@ +744,5 @@
>  
>          mSearchTerm = searchTerm;
> +
> +        if (handler != null) {
> +            mAutocompleteHandler = handler;

This is unnecessary as the auto complete handler can already be null. See how onFilter is called with null [1], which eventually calls filterEditingMode -> BrowserSearch.filter with null.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/ToolbarEditText.java?rev=5a5018a72c8d#216

Comment 9

[Ashish Madeti [:madeti]]

Attachment: Patch for bug 1039766

Comment 10

[Ashish Madeti [:madeti]]

(In reply to Michael Comella (:mcomella) from comment #8)
Thanks for the review, Michael.
I have updated the patch according to your suggestions.

> It works for me but I'm using a relatively quick device (Nexus 4). What
> device are you using? Do you have any special steps to reproduce?
Like you guessed, it is working for me too. It was slow because I was using the android emulator for testing.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://tbpl.mozilla.org/?tree=Try&rev=2edf487336e1

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8467525 [details] [diff] [review]
Patch for bug 1039766

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

lgtm.

If the tests go green (comment 11), feel free to add the `checkin-needed` keyword [1].

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/42638cf9dc4d

Thanks for the patch, Ashish! One request for the future, we usually prefer commit messages to explain what the patch is doing rather than just re-stating the problem it's fixing. Thanks again!
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/42638cf9dc4d

Comment 15

[Teodora Vermesan (:TeoVermesan)]

Device: Samsung Galaxy Nexus 
OS: Android 4.2.1

After pasting into the URL bar, the edit mode is displayed and the search engines are in the same order as in the Search settings page, so verified fixed on:
Build: Firefox for Android 34.0a1 (2014-08-07)