881780 - Make BrowserSearch inherit from HomeFragment

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

As it needs all the common context actions implemented in HomeFragment (introduced in bug 878136). This also means using HomeListView in BrowserSearch.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: (1/5) Make favicon and bookmark type optional in HomeContextMenuInfo

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: (2/5) Move touch handling from BookmarksPage to HomeListView

Comment 3

[Lucas Rocha (:lucasr)]

Attachment: (3/5) Move ContextMenu creation handling to HomeFragment

Comment 4

[Lucas Rocha (:lucasr)]

Attachment: (4/5) Merge item click listener into BookmarksPage

Comment 5

[Lucas Rocha (:lucasr)]

Attachment: (5/5) Change BrowserSearch to inherit from HomeFragment

Comment 6

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 762065 [details] [diff] [review]
(1/5) Make favicon and bookmark type optional in HomeContextMenuInfo

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

::: mobile/android/base/home/HomeListView.java
@@ +12,5 @@
>  import android.content.Context;
>  import android.database.Cursor;
>  import android.util.AttributeSet;
>  import android.view.ContextMenu.ContextMenuInfo;
> +import android.view.MotionEvent;

This doesnt seem to be needed here.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 762066 [details] [diff] [review]
(2/5) Move touch handling from BookmarksPage to HomeListView

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

The MotionEvent import should be a part of this. r+ with that.

Comment 8

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 762067 [details] [diff] [review]
(3/5) Move ContextMenu creation handling to HomeFragment

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

I hope the context menu logic is same for history and bookmark tabs.

Comment 9

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 762069 [details] [diff] [review]
(5/5) Change BrowserSearch to inherit from HomeFragment

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

::: mobile/android/base/BrowserSearch.java
@@ +48,5 @@
>  
>  /**
>   * Fragment that displays frecency search results in a ListView.
>   */
> +public class BrowserSearch extends HomeFragment implements AdapterView.OnItemClickListener,

Break this to next line please. Just like BrowserApp.

Comment 10

[Lucas Rocha (:lucasr)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #6)
> Comment on attachment 762065 [details] [diff] [review]
> (1/5) Make favicon and bookmark type optional in HomeContextMenuInfo
> 
> Review of attachment 762065 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/home/HomeListView.java
> @@ +12,5 @@
> >  import android.content.Context;
> >  import android.database.Cursor;
> >  import android.util.AttributeSet;
> >  import android.view.ContextMenu.ContextMenuInfo;
> > +import android.view.MotionEvent;
> 
> This doesnt seem to be needed here.

Removed.

Comment 11

[Lucas Rocha (:lucasr)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #7)
> Comment on attachment 762066 [details] [diff] [review]
> (2/5) Move touch handling from BookmarksPage to HomeListView
> 
> Review of attachment 762066 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The MotionEvent import should be a part of this. r+ with that.

Done.

Comment 12

[Lucas Rocha (:lucasr)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #8)
> Comment on attachment 762067 [details] [diff] [review]
> (3/5) Move ContextMenu creation handling to HomeFragment
> 
> Review of attachment 762067 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I hope the context menu logic is same for history and bookmark tabs.

Yeah. For now it seems to be working fine. If a fragment needs something different, we can always override onCreateContextMenu() for the custom bits.

Comment 13

[Lucas Rocha (:lucasr)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #9)
> Comment on attachment 762069 [details] [diff] [review]
> (5/5) Change BrowserSearch to inherit from HomeFragment
> 
> Review of attachment 762069 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/BrowserSearch.java
> @@ +48,5 @@
> >  
> >  /**
> >   * Fragment that displays frecency search results in a ListView.
> >   */
> > +public class BrowserSearch extends HomeFragment implements AdapterView.OnItemClickListener,
> 
> Break this to next line please. Just like BrowserApp.

Done.

Comment 14

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/projects/fig/rev/441bc08c4d28
http://hg.mozilla.org/projects/fig/rev/62f98d2e9508
http://hg.mozilla.org/projects/fig/rev/129890019632
http://hg.mozilla.org/projects/fig/rev/621adb50d7eb
http://hg.mozilla.org/projects/fig/rev/9da587767f31

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/441bc08c4d28
https://hg.mozilla.org/mozilla-central/rev/129890019632
https://hg.mozilla.org/mozilla-central/rev/621adb50d7eb
https://hg.mozilla.org/mozilla-central/rev/9da587767f31