Why don't you use visibility_monitor.js? That will let you pre-create the rows and keep the scrollbar. Also it will avoid duplicating the code on a per app basis for the same issues. I would like to understand the rationale before merging anything :)

Comment on attachment 751027 [details] Pointer to PR 9847 The code looks good. I agree with Vivien that using visibility monitor would be better though. Since it would enable us to bring back the scroll bar and limit the memory consumption (it can get pretty huge). So we should sort this out before landing anything.

Th 3 main reasons for not using visibility_monitor yet were: - Visibility monitor doesn't support nested child. We'll only receive events when a header is shown/hidden (Bug 865750). - Current implementation of edit mode, needs all the elements in the DOM in order to perform a "Delete all" - Partners want to see the call log speed up as soon as possible, in order to take some time measurements. That said, I totally agree we need to add visibility_monitor to our lists. What about creating another bug for improving memory performance, and land this one at it is right now? Thanks!

(In reply to Alberto Pastor [:albertopq] from comment #4) > Th 3 main reasons for not using visibility_monitor yet were: > > - Visibility monitor doesn't support nested child. We'll only receive events > when a header is shown/hidden (Bug 865750). David has started on it already or will start soon. I won't mind if this bug depends on bug 865750. > > - Current implementation of edit mode, needs all the elements in the DOM in > order to perform a "Delete all" > That's ok. The code for visibility_monitor.js will make you keep the container for each row. So you will still have all a container with an id. That should be enough right? > - Partners want to see the call log speed up as soon as possible, in order > to take some time measurements. > That's not a very technical argument :) Also since this bug is not leo+ while the underlying bug is I would assume that the partner would appreciate if priorities are taken into account ;P > That said, I totally agree we need to add visibility_monitor to our lists. > What about creating another bug for improving memory performance, and land > this one at it is right now? I fear that landing this one right now will make it fast for the call log but will de-prioritize the lower level bug that will try to provide some code to help all apps that use groups for list (like the call log but also the sms app, settings, music, etc..). Also Gecko history has proven that once something seems to work it stays for a long time (you wil lfind tons of old comments in Gecko that says: temporary hack!) > > Thanks! Sorry I don't want to slow you down, just want to make it the best way I can think of for the product.

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #5) > (In reply to Alberto Pastor [:albertopq] from comment #4) > > Th 3 main reasons for not using visibility_monitor yet were: > > > > - Visibility monitor doesn't support nested child. We'll only receive events > > when a header is shown/hidden (Bug 865750). > > David has started on it already or will start soon. I won't mind if this bug > depends on bug 865750. > > > > > - Current implementation of edit mode, needs all the elements in the DOM in > > order to perform a "Delete all" > > > > That's ok. The code for visibility_monitor.js will make you keep the > container for each row. So you will still have all a container with an id. > That should be enough right? Not really. It depends on the checkbox inside the container. Anyway, we could add more logic for handling that. > > > - Partners want to see the call log speed up as soon as possible, in order > > to take some time measurements. > > > > That's not a very technical argument :) Also since this bug is not leo+ > while the underlying bug is I would assume that the partner would appreciate > if priorities are taken into account ;P I didn't say they were all technical reasons :P . At the other hand, I think will be leo+ soon... > > > That said, I totally agree we need to add visibility_monitor to our lists. > > What about creating another bug for improving memory performance, and land > > this one at it is right now? > > I fear that landing this one right now will make it fast for the call log > but will de-prioritize the lower level bug that will try to provide some > code to help all apps that use groups for list (like the call log but also > the sms app, settings, music, etc..). > > Also Gecko history has proven that once something seems to work it stays for > a long time (you wil lfind tons of old comments in Gecko that says: > temporary hack!) > > > > Thanks! > > Sorry I don't want to slow you down, just want to make it the best way I can > think of for the product. I would say, depending on how long is going to take Bug 865750 to be fixed, we can land this and change it later, or wait. If David already started to work (it was just today!), I agree it makes no sense landing this to change it later :) Any time estimation for the visibility_monitor bug?

(In reply to Alberto Pastor [:albertopq] from comment #6) > I would say, depending on how long is going to take Bug 865750 to be fixed, > we can land this and change it later, or wait. If David already started to > work (it was just today!), I agree it makes no sense landing this to change > it later :) > > Any time estimation for the visibility_monitor bug? I can't answer for David. Also it would be nice if you can coordinate with him in order to ensure what he is doing fit with your use case! Let needinfo him to make sure he is aware of the work here.

I have started work on a generalized visibility monitor that will allow monitoring of grandchildren. It turns out to be non-trivial, and I don't have anything checked in. I'm heading to San Diego today and will be working on Leo media-related bugs Tuesday and Wednesday. But I hope to work on this one on the plane at least. (Its by far the most interesting bug I've got :-) Having a concrete use case like this is helpful. Thanks for letting me know about it. I'll let you know as soon as I have something ready to be tried out. And if I can't finish something on the plane today, I'll at least try to land a WIP patch so that someone can take over the bug if necessary.

Alberto, The visibility monitor extension I'm working on won't be a fully-general one. It will add the ability to monitor the grandchildren of the scrolling element instead of monitoring the children. I'll probably also add the ability to limit the elements monitored by the tagname of their parent, and possibly by the tagname of the elements themselves. So you could monitor the grandchildren of the list that are children of a div, for example. Or maybe monitor all li elements that are children of a div that are children of the list. Will this work for your use case here?

Hi David, Thanks for taking this. I would say that your description would work for us, but just to make sure, anything that allow us to know when each of the elements (the "li" tags) in the lists defined by the Building Blocks (http://buildingfirefoxos.com/building-blocks/lists/) are in the viewport, will work in for most of apps. Thanks!

Vivien, my understanding here after reading the comments is we will wait till bug 865750 is fixed, isnt?

(In reply to Francisco Jordano [:arcturus] from comment #11) > Vivien, my understanding here after reading the comments is we will wait > till bug 865750 is fixed, isnt? I would like too!

Etienne, Vivien, is dialer still planning to implement this now that the tag_visibility_monitor landed in bug 865750? I think in contacts we may have come to the conclusion that a streaming cursor approach may be more scalable. The issue with visibility_monitor is it wants most of the DOM in place in order to detect when elements are coming on screen. It seems keeping a minimal DOM is a bigger win even if we have to keep adding/removing.

Well if we investigated both approaches and found a winner I trust the perf team on that one :)

(In reply to Etienne Segonzac (:etienne) from comment #14) > Well if we investigated both approaches and found a winner I trust the perf > team on that one :) I don't know that its completely settled. The contacts API has held us back from really implementing/proving out that solution. We will probably need to migrate to Datastore before we can conclusively say which is better. I guess we were just wondering if you had plans around this for dialer or not. Thanks!

We are also looking at solutions for this in bug 940428.

This sounds like a feature request, not a perf issue. Once we have a good solution for infinite scrolling, if there are perf issues with that, then file a new bug and we'll look at it.

Anthony, I inherit this when Alberto left, I'm not putting enough love here but I'm sure you will. Cheers, F.

http://robert.ocallahan.org/2014/02/implementing-virtual-widgets-on-web.html is a good start when we'll implement this. Wilfred: can we put this in our backlog?

Let me also add that implementing this will provide better reactivity when opening the call log and use less memory.

backlog for now for v1.4. revisit when we have done v1.4