Closed Bug 881780 Opened 8 years ago Closed 8 years ago

Make BrowserSearch inherit from HomeFragment

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

(Whiteboard: fixed-fig)

Attachments

(5 files)

As it needs all the common context actions implemented in HomeFragment (introduced in bug 878136). This also means using HomeListView in BrowserSearch.
Blocks: 862794
Blocks: 871642
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.
Attachment #762065 - Flags: review?(sriram) → review+
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.
Attachment #762066 - Flags: review?(sriram) → review+
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.
Attachment #762067 - Flags: review?(sriram) → review+
Attachment #762068 - Flags: review?(sriram) → review+
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.
Attachment #762069 - Flags: review?(sriram) → review+
(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.
(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.
(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.
(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.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.