513471 - Failures by editing a card with a custom photo

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Richard Marti (:Paenglab)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; de-DE; rv:1.9.3a1pre) Gecko/20090826 Minefield/3.7a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20090829 Lightning/1.0pre Shredder/3.1a1pre

When a custom photo is assigned to a card and I edit some entry and I then want to close this card with OK nothing happens. After close with Cancel and reopen the card no custom image is shown. Now I can select 'Generic Photo' but this will not be saved. Only after reassigning of the photo, I can switch to 'Generic Photo' and save this.

This happens with photos from 'On this Computer' and from 'On the Web'.

I tested it also with 3.0b4pre and had the same behavior.

Reproducible: Always

Steps to Reproduce:
1. Open a card and change to Photo Tab
2. Assign a image with the 'Browse' button 
3. Close the card with the 'OK' button
4. Reopen the card and edit one field on the 'Contact' Tab
5. Close the card with the 'OK' button
Actual Results:  
Card doesn't close. After closing the card through the 'Cancel' button and reopening the card in the error console appears this error: 

Error: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]
Source file: chrome://messenger/content/addressbook/abCardOverlay.js
Line: 462

The image is no more shown in the 'Photo' Tab, but the changes on the 'Contact' Tab are saved.


Expected Results:  
The card closes and all is correctly saved

By opening of the address book and the card I see also this warnings. Not beautiful but also not evil.

Warning: XUL box for hbox element contained an inline img child, forcing all its children to be wrapped in a block.
Source file: chrome://messenger/content/addressbook/abEditCardDialog.xul
 ----------
Warning: XUL box for hbox element contained an inline img child, forcing all its children to be wrapped in a block.
Source file: chrome://messenger/content/addressbook/addressbook.xul

Comment 1

[[:Aureliano Buendía]]

Confirm here on 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090829 Lightning/1.0pre Shredder/3.0b4pre ID:20090829032304

It happens also if 

4. Reopen the card 
5. Close the card with the 'OK' button

Comment 2

[[:Aureliano Buendía]]

Another information: if I add a new photo with browsing on my computer and click OK the photo is "saved". But when I reopen and I can't click OK but only CANCEL the photo is removed!

Comment 3

[Josh Geenen (:pi)]

I can reproduce this in the nightlies of latest-comm-trunk and latest-comm-1.9.1 using Windows Vista and will take a look ASAP.  It seemed to work fine in Linux but I'd like to confirm that with my laptop.

If I had to guess I'd say the cause is abCardOverlay.js
http://mxr.mozilla.org/comm-central/source/mail/components/addrbook/content/abCardOverlay.js#578
577   if (type == "file" && document.getElementById("PhotoFile").file)
578     photoURI = "file://" + document.getElementById("PhotoFile").file.path;
That isn't how a file URI should be obtained and would cause the NS_NOINTERFACE error mentioned in post 1.  Windows' slashes are in the opposite direction than URI's...That would explain why it works in Linux (and probably Mac OS).

Steps to Reproduce:
1. In Windows, create a new contact and fill in at least one required field
2. Go to the Photo tab, click Browse, and find and open an image
3. Click OK on the New Contact dialog
4. Edit that new contact
5. Click OK

Expected:
Contact is saved and dialog is closed

Actual:
The dialog stays open

Comment 4

[Josh Geenen (:pi)]

Attachment: Patch

This patch should fix the problem caused by the code mentioned in my previous comment and replaces that with nsIIOService.

It also doesn't remove the previous photo until the new one is saved in case a problem occurs in savePhoto in the future.

Comment 5

[[e-mail]]

Attachment: First round of fixes

This fixes most of my problems from the original patch, although there are still some issues which I will see if I can address in a follow-up patch.

Comment 6

[[e-mail]]

Comment on attachment 398651 [details] [diff] [review]
First round of fixes

Bah, :pi is ambiguous :-(

Comment 7

[David :Bienvenu]

Comment on attachment 398651 [details] [diff] [review]
First round of fixes

this fixes it for me on Windows w/ TB.

Comment 8

[Josh Geenen (:pi)]

Comment on attachment 398651 [details] [diff] [review]
First round of fixes

It fixes the problem (and improves several other things) and works great in Vista and Linux.

>diff -r 85b424089b9f mailnews/addrbook/content/abCardOverlay.xul
>--- a/mailnews/addrbook/content/abCardOverlay.xul	Thu Sep 03 22:35:04 2009 +0100
>+++ b/mailnews/addrbook/content/abCardOverlay.xul	Fri Sep 04 15:01:37 2009 +0100
> <overlay id="editcardOverlay"
>-     xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>-     xmlns:html="http://www.w3.org/1999/xhtml">
>+     xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
...
>-              <html:img align="center" src="" id="photo"
>-                        style="max-width: 25ch; max-height: 25ch; min-width: 1ch;"/>
...
>+        <vbox align="center" style="min-width: 25ch; max-width: 25ch;">
>+          <image id="photo" style="max-width: 25ch; max-height: 25ch;"/>
>+        </vbox>

The html img tag was there since XUL image elements don't always scale images correct (screenshot coming in a minute).

Comment 9

[Josh Geenen (:pi)]

Attachment: Screenshot of XUL image scaling

Shows how the XUL image tag scales a wide image (http://www.mozillamessaging.com/en-US/thunderbird/) vs the HTML img tag in the contact view pane.

Comment 10

[[e-mail]]

Attachment: Back to HTML image element

Comment 11

[Josh Geenen (:pi)]

Comment on attachment 398952 [details] [diff] [review]
Back to HTML image element

It looks good, thanks for doing this

Comment 12

[[e-mail]]

Pushed changeset f07bc939cb26 to comm-central.