Steps to reproduce: 1. Install https://addons.mozilla.org/en-US/android/addon/pocket-panel/ Actual results: - For a few seconds the message appears "No content could be find for this panel" and after that, the panel is blank. Only after rotating the phone from portrait to landscape, tapping or try scrolling down the list, the feeds appear. Expected results: - The panel should be immediately populated with Pocket reading feeds.

I've seen this happen with other panels. It sounds like there's some regression we need to track down... I can try to make a reduced testcase.

What's the regression exactly?

If this is a regression can you help find a window Teodora?

On the 04/25 build, installing pocket panel the panel displays "No content could be found for this panel" On the 04/26 build, installing pocket panel the panel displays "No content could be found for this panel" for a few seconds and after that it's blank. From the changeset: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b836d89be72b&tochange=0e91262606a6 is the Bug 1000616 the cause?

In this range I see these suspects: Bug 1000849 - Make a dummy MatrixCursor to listen for dataset changed notifications. r=lucasr http://hg.mozilla.org/mozilla-central/rev/528ed2029bc2 Bug 1000616 - Fix clicks on items when empty views and panel views are swapped. r=lucasr http://hg.mozilla.org/mozilla-central/rev/548827cce8d7

Thanks for getting the regression range. I can dig into this today.

(In reply to Lucas Rocha (:lucasr) from comment #5) > Bug 1000849 - Make a dummy MatrixCursor to listen for dataset changed > notifications. r=lucasr > http://hg.mozilla.org/mozilla-central/rev/528ed2029bc2 Before this patch landed, the empty view would have just stayed until the home pager was reloaded, so this isn't a regression, so much as a failure to properly fix that bug. I added some logging to investigate, and this is what I see happening: 1) We do a query when the panel is first added, there is no data yet, so we show the empty view as expected. 2) When data is saved, we observe a content change notification as expected, create a panel view, and call replacePanelView as expected, but nothing appears on the screen. 3a) The screen stays blank until it is touched, at which point the panel view content appears (no data change is happening during this process). 3b) Alternately, if another data changed notification fires while the screen is still blank, we follow the query logic again, and this causes the list to appear. This makes me think that we're doing something wrong with the view swapping. Any ideas?

(In reply to :Margaret Leibovic from comment #7) > (In reply to Lucas Rocha (:lucasr) from comment #5) > > > Bug 1000849 - Make a dummy MatrixCursor to listen for dataset changed > > notifications. r=lucasr > > http://hg.mozilla.org/mozilla-central/rev/528ed2029bc2 > > Before this patch landed, the empty view would have just stayed until the > home pager was reloaded, so this isn't a regression, so much as a failure to > properly fix that bug. > > I added some logging to investigate, and this is what I see happening: > > 1) We do a query when the panel is first added, there is no data yet, so we > show the empty view as expected. > 2) When data is saved, we observe a content change notification as expected, > create a panel view, and call replacePanelView as expected, but nothing > appears on the screen. > 3a) The screen stays blank until it is touched, at which point the panel > view content appears (no data change is happening during this process). > 3b) Alternately, if another data changed notification fires while the screen > is still blank, we follow the query logic again, and this causes the list to > appear. > > This makes me think that we're doing something wrong with the view swapping. Yep, sounds like it. From your description, this is looking like an invalidation issue i.e. the new view is not being drawn on screen after being added to the hierarchy. The addView()/removeView() calls in replacePanelView() should trigger a layout request in their parent which, in theory, would take care of the invalidation for us. We might be hitting an Android bug here. Have you tried to reproduce this bug in devices with different Android versions? Things to try in replacePanelView(): - Reverse the order of the addView()/removeView() calls. - Call parent.requestLayout() and/or parent.invalidate() after the addView/removeView calls.

(In reply to Lucas Rocha (:lucasr) from comment #5) > In this range I see these suspects: > > Bug 1000849 - Make a dummy MatrixCursor to listen for dataset changed > notifications. r=lucasr > http://hg.mozilla.org/mozilla-central/rev/528ed2029bc2 > > Bug 1000616 - Fix clicks on items when empty views and panel views are > swapped. r=lucasr > http://hg.mozilla.org/mozilla-central/rev/548827cce8d7 I just realized that both these changes were uplifted to beta, but yet I can't reproduce on beta. I wonder if there's some other patch that's on 31 but not 30 that's causing this strange interaction.

I ran a diff between mozilla-central and mozilla-beta to see what could be different, and the main difference is that the auth view logic in DynamicPanel isn't on beta. However, I can't find any obvious problems in there. It could very well be some bad interaction that is tickling an Android bug. I'll keep digging and experimenting.

(In reply to Lucas Rocha (:lucasr) from comment #8) > Yep, sounds like it. From your description, this is looking like an > invalidation issue i.e. the new view is not being drawn on screen after > being added to the hierarchy. The addView()/removeView() calls in > replacePanelView() should trigger a layout request in their parent which, in > theory, would take care of the invalidation for us. We might be hitting an > Android bug here. Have you tried to reproduce this bug in devices with > different Android versions? No, I haven't tested on different devices. I've just been testing on a Nexus 4. > Things to try in replacePanelView(): > - Reverse the order of the addView()/removeView() calls. > - Call parent.requestLayout() and/or parent.invalidate() after the > addView/removeView calls. None of these things have worked, and I'm having a hard time figuring out what's going wrong :( The view is definitely being re-drawn when the empty view is removed (hence the blank screen), but for some reason the PanelListView isn't appearing. I started digging into PanelListView a bit, but I don't have more time to investigate today. But it's weird that this isn't an issue when the auth stuff isn't in the tree. It does make me suspicious of some bigger view hierarchy issue.

Aha! I was wrong about the auth view being the only thing that's different. Pull-to-refresh is in 31, but not 30, and I found that's what's causing the problem. If I just comment out this if statement, the bug goes away: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/PanelLayout.java#397 This makes me feel better, since it makes a lot more sense that the problem would stem from here (rather than the DynamicPanel layout). I think the changesets in the regression range revealed this issue because before the patch for bug 1000849 landed, we would never replace the empty view with the panel layout. GeckoSwipeRefreshLayout is probably doing something wrong, but that's annoying to debug since jdover took that from a third party library :(

I've been debugging GeckoSwipeRefreshLayout, but I still can't seem to find the source of this problem. In onMeasure/onLayout, GeckoSwipeRefreshLayout seems to be setting the correct measurements for the child view, but yet it's somehow not being drawn during the draw() call. I'm really banging my head against the wall here, I think I need another set of eyes to look at this. Lucas, can you help me?

I am able to reproduce again bug 1000849 on latest 31 Aurora and 32 Nightly 02/06, but not 30 Beta 10

(In reply to :Margaret Leibovic from comment #13) > I've been debugging GeckoSwipeRefreshLayout, but I still can't seem to find > the source of this problem. In onMeasure/onLayout, GeckoSwipeRefreshLayout > seems to be setting the correct measurements for the child view, but yet > it's somehow not being drawn during the draw() call. > > I'm really banging my head against the wall here, I think I need another set > of eyes to look at this. Lucas, can you help me? Sure, I'll have at it today or tomorrow and post my findings here.

I'm going to prepare a patch for this bug to just disable pull-to-refresh for hub panels, since Firefox 31 is hitting beta really soon. Also, we haven't been 100% happy with the UX for this pull-to-refresh layout, so we could also take this time to refine that before we ship it in beta/release. I'll also update the MDN docs to indicate that the onrefresh API is still under development.

(In reply to :Margaret Leibovic from comment #16) > I'm going to prepare a patch for this bug to just disable pull-to-refresh > for hub panels, since Firefox 31 is hitting beta really soon. > > Also, we haven't been 100% happy with the UX for this pull-to-refresh > layout, so we could also take this time to refine that before we ship it in > beta/release. > > I'll also update the MDN docs to indicate that the onrefresh API is still > under development. No luck finding out what's going on here yet. I'll continue investigation tomorrow.

I think we should land this on aurora. If we can find a low-risk fix for this bug to uplift to beta, we can re-enable this with that fix. As much as the UX isn't amazing, pull-to-refresh is really useful, so if it weren't for this bug, I would have wanted to keep it enabled. Bug 1014335 was already filed about making the UX better.

Hmm... You can reproduce this even if you just make PanelRefereshLayout extend a generic FrameLayout, so I don't think its the SwipeView's fault here. Something doesn't like that this is wrapped...

So even though I see all the draw calls here, I never see any getView calls in the adapter. I dug in there a bit, and adding an adapter.notifyDatasetChanged() call after swapCursor() here: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/PanelListView.java#62 fixes this bug actually, but with a little logging, notifyDatsetChanged is actually called (as you'd expect) from swapCursor anyway. I'm not sure why the double call helps, or how having PanelRefreshLayout proxying in the middle causes it to break....

Comment on attachment 8434305 [details] [diff] [review] Disable pull-to-refresh Review of attachment 8434305 [details] [diff] [review]: ----------------------------------------------------------------- Give me just another day to see if I can figure this out.

(In reply to Lucas Rocha (:lucasr) from comment #21) > Comment on attachment 8434305 [details] [diff] [review] > Disable pull-to-refresh > > Review of attachment 8434305 [details] [diff] [review]: > ----------------------------------------------------------------- > > Give me just another day to see if I can figure this out. Okay, I'm going to request approval for Aurora in the mean time. And if we do land this before the merge, we can always back it out at the same time we uplift a solution.

Comment on attachment 8434305 [details] [diff] [review] Disable pull-to-refresh Note: We don't want to land this on Nightly, we just want to land it on Aurora before the merge so that this bug doesn't make it to beta. [Approval Request Comment] Bug caused by (feature/regressing bug #): a combination of things, but mostly bug 970707 User impact if declined: dynamic panels can be totally blank Testing completed (on m-c, etc.): tested locally Risk to taking this patch (and alternatives if risky): low-risk, disables new pull-to-refresh feature String or IDL/UUID changes made by this patch: none

Hit this on Nightly (06/11) after installing Goal.com. Just visiting about:home after installation and the view needed a tap to draw all it's data. ↳ (logcat/image, https://gist.github.com/AaronMT/a73390633dbaeb2689dc)

Installing the Goal.com add-on works fine on Firefox 30, Firefox 31 Beta 1, but 32 Aurora and 33 Nightly are still affected.

Margaret, Lucas: Ping?

I've run out of time to dig into this before going on PTO. Lucas, can you help take this over?

Comment on attachment 8450169 [details] [diff] [review] Force layout on child once swipe refresh layout is re-attached (r=mfinkle) For some reason, SwipeRefreshLayout doesn't do a proper layout pass on its child once it gets reattached. This patch simply ensures the child will get measured and laid out on the next layout pass. Not entirely sure why this is happening. This patch is unlikely to cause side effects so let's land it in Nightly and see how it goes. I tried it in a few different devices and seems to fix the bug.

Comment on attachment 8450169 [details] [diff] [review] Force layout on child once swipe refresh layout is re-attached (r=mfinkle) Approval Request Comment [Feature/regressing bug #]: Bug 970707 [User impact if declined]: Blank panel in about:home after installing an panel add-on e.g. World Cup feed add-on. [Describe test coverage new/current, TBPL]: Tested locally with a few different devices. Let's bake this in Nightly for a few days then uplift. [Risks and why]: Very low, this is just ensuring a layout pass happens on the panel contents after it gets attached to the view tree. [String/UUID change made/needed]: n/a

Tested with: Device: LG Nexus 4 OS: Android 4.4.2 Build: Firefox for Android 33.0a1 (2014-07-07) Installing goal.com and pocket-panel, the message "No content could be found for this panel" appears for about two seconds and after that the feeds appear.

Comment on attachment 8450169 [details] [diff] [review] Force layout on child once swipe refresh layout is re-attached (r=mfinkle) I haven't heard of any fallout on Nightly. Aurora+

Tested with: Device: LG Nexus 4 OS: Android 4.4.2 Build: Firefox for Android 32.0a2 (2014-07-14) Installing goal.com and pocket-panel, the message "No content could be found for this panel" appears for about two seconds and after that the feeds appear. Marking the bug as Verified Fixed.