Closed Bug 943848 Opened 11 years ago Closed 10 years ago

[Contacts] Contacts images are rendered too late

Categories

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Contacts
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 1089538

People

(Reporter: vingtetun, Unassigned)

Details

With APZ turned on the code from apps/communications/contacts/js/utilities/image_loader.js does not works very well anymore, mostly because the current view is out of sync with the current scrolling position, while image_loader.js use the scroll events to update image visibility. I would have hope that with bug 847223 and bug 542158 we don't need that anymore. But one thing that we still do is to download images that are out of view. I think that is fine if the images does not point to a remote source, but we need to check that.
Jose, we got this discussion 1 year and a half ago so I forgot the details. Does the images for facebook contacts (or any other providers) are saved locally and updated only when we sync with the provider?
Flags: needinfo?(jmcf)
FYI, I recently tried removing image_loader.js in favor of rendering the images directly in the visibility_monitor onscreen callback. This produced quite janky scrolling, though. But I thought the whole point of image_loader.js was to wait until scrolling had completed to load images. It seems if APZ sends an onscroll event when scrolling completes, that should still be possible.
(In reply to Ben Kelly [:bkelly] from comment #2) > FYI, I recently tried removing image_loader.js in favor of rendering the > images directly in the visibility_monitor onscreen callback. This produced > quite janky scrolling, though. > Do you know why scrolling were janky ? > But I thought the whole point of image_loader.js was to wait until scrolling > had completed to load images. It seems if APZ sends an onscroll event when > scrolling completes, that should still be possible. Maybe the only issue then it my misunderstanding of this code. But I wonder if we still need it, and if we do, what is missing in the platform to be able to remove it.
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #3) > (In reply to Ben Kelly [:bkelly] from comment #2) > > FYI, I recently tried removing image_loader.js in favor of rendering the > > images directly in the visibility_monitor onscreen callback. This produced > > quite janky scrolling, though. > > > > Do you know why scrolling were janky ? Well, I think it was due to trying to load images while scrolling vs waiting for 100ms after scrolling stops. I did not profile it since I solved the particular problem I was looking at another way.
In all cases. I think that's fine if this bug does not block the [meta] for scroll handlers since the current behavior is the same. I was just thinking that it is buggy :) I would like to keep this bug opened though in order to investigate if we really need this little script anymore, and if yes, what is missing in the platform.
(In reply to Ben Kelly [:bkelly] from comment #2) > FYI, I recently tried removing image_loader.js in favor of rendering the > images directly in the visibility_monitor onscreen callback. This produced > quite janky scrolling, though. > > But I thought the whole point of image_loader.js was to wait until scrolling > had completed to load images. It seems if APZ sends an onscroll event when > scrolling completes, that should still be possible. We introduced the image loader in the past because the contacts app had memory problems if the images of all contacts were loaded. On the other hand, the image loader helps to make things faster for Facebook as we only load Facebook data when a FB Contact is onscreen. Other approaches in the past caused memory problems. Probably as Ben said this could be solved with the visibility monitor but in any case I think loading FB Data and images on demand is necessary.
Flags: needinfo?(jmcf)
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #1) > Jose, we got this discussion 1 year and a half ago so I forgot the details. > Does the images for facebook contacts (or any other providers) are saved > locally and updated only when we sync with the provider? FB Images are stored right now in a Datastore. Yes, they are updated when we sync with the provider
(In reply to Jose M. Cantera from comment #6) > (In reply to Ben Kelly [:bkelly] from comment #2) > > FYI, I recently tried removing image_loader.js in favor of rendering the > > images directly in the visibility_monitor onscreen callback. This produced > > quite janky scrolling, though. > > > > But I thought the whole point of image_loader.js was to wait until scrolling > > had completed to load images. It seems if APZ sends an onscroll event when > > scrolling completes, that should still be possible. > > We introduced the image loader in the past because the contacts app had > memory problems if the images of all contacts were loaded. Which is exactly what the platform has been fixed for. > On the other hand, the image loader helps to make things faster for Facebook > as we only load Facebook data when a FB Contact is onscreen. By facebook Data do you mean that you're loading something else than images ? Sorry if I missed something obvious. > Other > approaches in the past caused memory problems. Probably as Ben said this > could be solved with the visibility monitor visibility_monitor.js will have some issue soon with asynchronous pan/zoom beeing the default scroll heuristic for apps. That what I don't want to rely on it. > but in any case I think loading > FB Data and images on demand is necessary. I think we are on the same line here. But the platform has been fixed in some ways that it should be able to load your images only for the visible area of the screen. So now I would like to understand a bit more about what you call 'Data'. Is there anything else than images that are loaded by image_loader.js ?
Flags: needinfo?(jmcf)
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #8) > So now I would like to understand a bit more about what you call 'Data'. Is > there anything else than images that are loaded by image_loader.js ? The FB data is loaded entirely by the collaboration between image_loader.js and fb_resolver.js. See https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/fb_resolver.js#L20 We render the image and other information such as the Organization name https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/fb_resolver.js#L51
Flags: needinfo?(jmcf)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.