Closed
Bug 958171
Opened 11 years ago
Closed 11 years ago
HomePager doesn't handle live config changes properly
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: lucasr, Assigned: lucasr)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Margaret
:
review+
|
Details | Diff | Splinter Review |
Gets into a bad state where the title strip doesn't match the current page and the panels don't load their content on swipe.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Comment on attachment 8358495 [details] [diff] [review]
Fix adapter update logic for when HomeConfig changes (r=margaret)
ViewPager (and, more specifically, the fragment-based ones) is a bit weird in that the fragments are owned by the view, not the adapter. So, a notifyDataSetChanged() will not necessary do the right thing for us e.g. destroy existing fragments, force recreation, etc.
So, with this patch, we
1) force fragment destruction by setting a null adapter on the pager
2) Update the existing adapter with the new list of panel configs
3) Re-install the adapter with the final state
4) Set the pager position based on the configured default panel
This patch needs to be rebased on top of changes from bug 958185.
Attachment #8358495 -
Flags: review?(margaret.leibovic)
Comment 3•11 years ago
|
||
Comment on attachment 8358495 [details] [diff] [review]
Fix adapter update logic for when HomeConfig changes (r=margaret)
Review of attachment 8358495 [details] [diff] [review]:
-----------------------------------------------------------------
Tricky, but your explanation makes sense.
I wanted to ask if we still need the setAdapter() call in show(), but if we get rid of that we'll need some other way to get at the adapter we create there. Maybe we should be creating the adapter in updateUiFromPageEntries? But if we do that we'll need the FragmentManager. I think this patch is fine, I'm just thinking aloud here :)
Attachment #8358495 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to :Margaret Leibovic from comment #3)
> Comment on attachment 8358495 [details] [diff] [review]
> Fix adapter update logic for when HomeConfig changes (r=margaret)
>
> Review of attachment 8358495 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Tricky, but your explanation makes sense.
>
> I wanted to ask if we still need the setAdapter() call in show(), but if we
> get rid of that we'll need some other way to get at the adapter we create
> there. Maybe we should be creating the adapter in updateUiFromPageEntries?
> But if we do that we'll need the FragmentManager. I think this patch is
> fine, I'm just thinking aloud here :)
Yeah. The main reason why we have to create the adapter in show() is that we'll have to set the canLoadHint depending on the given animation.
Comment 6•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•