904840 - [Fig] History page is inflated during startup

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Sriram Ramasubramanian [:sriram]]

During startup, the first page shown is bookmarks page. However, the history page is inflated even when it is not going to be shown.

Comment 1

[Sriram Ramasubramanian [:sriram]]

From my preliminary analysis, since history page is the first element in the list, its always loaded -- as the userVisibilityHint is true. Even after setting the current item to be bookmarks page, the adapter doesn't set the userVisibilityHint of history page to be false.

But if the first page is made to be History page, the bookmarks and reading list aren't loaded until user navigates to them. Their user visibility hint is false until then.

Comment 2

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Ooops. It was such a small thing to move everything to load(). And I investigated so much. :coffee-time:

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 789825 [details] [diff] [review]
Patch

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

::: mobile/android/base/home/HistoryPage.java
@@ +51,5 @@
>          // Show most visited page as the initial page.
>          // Since we detach/attach on config change, this prevents from replacing current fragment.
>          if (!initializeVisitedPage) {
>              showMostVisitedPage();
>              initializeVisitedPage = true;

Not in your patch but: this should be renamed to mInitializeVisitedPage. Please file a follow-up.

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 789825 [details] [diff] [review]
Patch

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

Just flipping the review flag to ensure you'll see this.

::: mobile/android/base/home/HistoryPage.java
@@ +42,5 @@
>          mTabWidget.addTab(R.drawable.icon_most_recent, R.string.home_most_recent_title);
>          mTabWidget.addTab(R.drawable.icon_last_tabs, R.string.home_last_tabs_title);
>  
> +        mTabWidget.setTabSelectionListener(this);
> +        mTabWidget.setCurrentTab(mSelectedTab);

You need to call loadIfVisible() here. Otherwise the history page won't load if it's the first visible page (when you tap the URL bar).

Comment 5

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 789825 [details] [diff] [review]
Patch

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

Based on IRC conversation, will land this with loadIfVisible() added there.

Comment 6

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/projects/fig/rev/c6e9046906ca

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c6e9046906ca