Closed Bug 904840 Opened 8 years ago Closed 8 years ago

[Fig] History page is inflated during startup

Categories

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

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: sriram, Assigned: sriram)

References

Details

(Whiteboard: fixed-fig)

Attachments

(1 file)

During startup, the first page shown is bookmarks page. However, the history page is inflated even when it is not going to be shown.
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.
Attached patch PatchSplinter Review
Ooops. It was such a small thing to move everything to load(). And I investigated so much. :coffee-time:
Attachment #789825 - Flags: review?(lucasr.at.mozilla)
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.
Attachment #789825 - Flags: review?(lucasr.at.mozilla) → review+
Blocks: 871651
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).
Attachment #789825 - Flags: review+ → review-
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.
Attachment #789825 - Flags: review- → review+
https://hg.mozilla.org/mozilla-central/rev/c6e9046906ca
Assignee: nobody → sriram
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.