Open Bug 530462 Opened 15 years ago Updated 2 years ago

Large contact photos "On the Web" are not fully downloaded

Categories

(MailNews Core :: Address Book, defect)

defect

Tracking

(Not tracked)

People

(Reporter: pi, Unassigned)

References

Details

(Whiteboard: [patchlove][has draft patch])

Attachments

(1 file, 1 obsolete file)

The method used to download and save online photos does not work correctly with large photos and/or slow connections.  I originally found this problem with my extension, which can download contact photos from Google.

I think the problem is that the input stream isn't ready to be read.  istream.available() only returns what is ready; not the total number of bytes.  In my extension, inserting a setTimeout to call a second function that writes to the output stream [0] seems to work, but photos are compressed to a few KBs.

Steps to reproduce:
1. Find or upload a large online photo (a few MBs)
2. Create a new contact and go to the Photo tab
3. Paste the photo URL to the textbox under "On the Web" but don't click Update.
4. Click OK

Results:
After waiting (clicking OK again will just create multiple contacts) nothing shows up the contact view pane, and opening the edit contact dialog shows the generic photo with "On the Web" checked and the original URL.

Only the first bytes have been written, so if you look at the photo in your profile directory the rest of the photo is black or something random.

[0] https://www.mozdev.org/bugs/show_bug.cgi?id=20901
It looks like wrapping buffer.writeFrom inside of a while loop fixes the issue.
Assignee: nobody → joshgeenen+bugzilla
Status: NEW → ASSIGNED
Attachment #424437 - Flags: review?(bugzilla)
I forgot to add the fix to Seamonkey in the previous patch.
Attachment #424437 - Attachment is obsolete: true
Attachment #424438 - Flags: review?
Attachment #424437 - Flags: review?(bugzilla)
Attachment #424438 - Flags: review? → review?(bugzilla)
Comment on attachment 424438 [details] [diff] [review]
Patch 1.1 - Also adds the fix for Seamonkey

Hmm, so according to the documentation savePhoto shouldn't be using nsIChannel.open as that blocks the UI thread. I'm not quite sure whether we should switch to async or not.

Therefore I'm going to defer to Neil to get his thoughts.
Attachment #424438 - Flags: review?(bugzilla) → review?(neil)
I believe I commented on that aspect in the original bug.
(Sorry but I don't remember the bug# for it.)
(In reply to comment #4)
> I believe I commented on that aspect in the original bug.
> (Sorry but I don't remember the bug# for it.)

See Bug 119459 comment 118 and the end of 117 (quoted in 118)
Well, you could use a persist object, or, at a slightly lower level, asyncCopy in NetUtils.jsm, although they requires some extra processing if you want to allow for copy errors. I don't think such a thing as a good sync copier exists.
Comment on attachment 424438 [details] [diff] [review]
Patch 1.1 - Also adds the fix for Seamonkey

I guess this is better than nothing.
Attachment #424438 - Flags: review?(neil) → review+
there has been no activity for more than a year, do you still want to add this patch? if not, i would write an async download. i think it would also make sense to downscale the images before saving to save disk space.
(In reply to comment #8)
> there has been no activity for more than a year, do you still want to add
> this patch? if not, i would write an async download. i think it would also
> make sense to downscale the images before saving to save disk space.
(In reply to comment #7)
> Comment on attachment 424438 [details] [diff] [review] [review]
> Patch 1.1 - Also adds the fix for Seamonkey
> 
> I guess this is better than nothing.

You can go ahead and take it
Assignee: joshgeenen+bugzilla → nobody
Status: ASSIGNED → NEW
Whiteboard: [patchlove][has draft patch]
Depends on bug 892889
Depends on: 892889
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: