Implement "tabs from last session" sub-fragment in the new about:home

RESOLVED FIXED in Firefox 26

Status

()

defect
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: lucasr, Assigned: lucasr)

Tracking

unspecified
Firefox 26
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-fig)

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Tapping on the "Tabs from last session" button should open a new fragment inside homepager.
(Assignee)

Comment 1

6 years ago
Attachment #767739 - Flags: review?(wjohnston)
(Assignee)

Comment 2

6 years ago
Attachment #767743 - Flags: review?(bnicholson)
(Assignee)

Comment 3

6 years ago
FYI: Again, the code to actually show the fragment is coming in a separate patch. This one is self-contained enough to be reviewed separately.
(Assignee)

Updated

6 years ago
Blocks: 887268
(Assignee)

Updated

6 years ago
Blocks: 887269
(Assignee)

Updated

6 years ago
Blocks: 887270
(Assignee)

Updated

6 years ago
Depends on: 882715
Drive-by: Isn't "Tabs from last time" supposed to be a sub-section of the "Visited" page? Or has that changed? If not, I feel like we shouldn't put "Page" in the name for this fragment.
Comment on attachment 767743 [details] [diff] [review]
Implement "Last Tabs" page for new about:home

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

::: mobile/android/base/home/LastTabsPage.java
@@ +126,5 @@
> +    @Override
> +    public void onActivityCreated(Bundle savedInstanceState) {
> +        super.onActivityCreated(savedInstanceState);
> +
> +        // Intialize adapter

Nit: s/Intialize/Initialize/

@@ +131,5 @@
> +        mAdapter = new LastTabsAdapter(getActivity());
> +        mList.setAdapter(mAdapter);
> +
> +        // Create callbacks before the initial loader is started
> +        mCursorLoaderCallbacks = new CursorLoaderCallbacks();

Does mCursorLoaderCallbacks need to be a class variable? This is the only place you use it, so you could probably make it local unless you planned on referencing it elsewhere.
Attachment #767743 - Flags: review?(bnicholson) → review+
Priority: -- → P1
Comment on attachment 767739 [details] [diff] [review]
Add getFaviconBytesForUrl() to BrowserDB

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

::: mobile/android/base/db/LocalBrowserDB.java
@@ +685,5 @@
> +                         Combined.URL + " = ?",
> +                         new String[] { uri },
> +                         null);
> +
> +            if (!c.moveToFirst()) {

cr.query can return null.
Attachment #767739 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/c4a0d7678b34
https://hg.mozilla.org/mozilla-central/rev/950e12ef1eb8
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.