1182460 - Exporting/saving address book with "All address books" selected exports only headers, or 0 byte file

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Anje]

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150630154324

Steps to reproduce:

Open 'Address Book'
'All Address Books' is by default selected.
Tools > Export  is enabled, so select it.
Select where to save, give suitable name, save as .csv file.



Actual results:

.csv file is saved.
Open using eg: Excel
File is read only.
File contains headers only - no data.
File cannot be deleted until the actual 'Address Book' window is closed.


Expected results:

File should not be read only.
File should contain data
File should be able to be deleted regardless of whether the Thunderbird Address Book is open.

I would like to suggest:
Either the option to use 'All Address Books' as able to Export as .csv as per all other individual address books 
OR the option to Export the 'All Address Books' address book should be removed.

I tested this after getting confused Support queries and discovered the above.

Comment 1

[Anje]

Update and correction on first comment.
'All Address Books' is by default selected.
Tools > Export  is enabled, so select it.
Select where to save, give suitable name, save as .csv file.

File was not read only, but it was empty.
So should this option be working or not available?


However, right click on 'All Address Books' and select 'Morefunctionsforaddressbook' > Export > as .csv
This method caused the read only, empty file which could not be deleted until Address Book was closed.
I'll send email to Paolo Kaosmos on this.

Comment 2

[Thomas D. (:thomas8)]

My expectation would be to allow exporting from "All Address Books" and fix things so that it works. If that's hard, temporarily disable until we can get it to work. Shouldn't be too hard. Consider adding Address book column in the export if it's not there yet, to allow distinguishing duplicate entries from different ABs.

Comment 3

[Matt]

I think that as the default selection is used when the menu is used that there is little option but to either fix the export to include ALL address books.  

In the interim perhaps the option on the menu could be disabled as suggested by Thomas.  But that will only lead to the reverse user issue.

Now they ask.  I get and empty file. why?
Then they will ask. The option is greyed out.  How do I make it active?

Perhaps the dirty fix may be to place the export option on the context menu for the address books and suppress the export option for the root.  

My experience with the read only is different to Anje however.  I use Libre Office and do not see any read only lock on the file. Deletion was not an issue either.

As there is little real difference between Vista and Windows 7 I would therefore assume at this point the difference may be external to Thunderbird.

Comment 4

[John Veness]

I came across this issue today. Tried to export someone's address book while All Address Books was selected (the default), trying to export as LDIF, result was a zero-byte file. Someone might want to change the title of the bug to say something like "exports no data" instead of "exports only headers", as in the LDIF case, not even headers were exported, just a 0-byte file.

The obvious user expectation when All Address Books is selected is to export all addresses. If that can't be fixed quickly, instead of greying out the Export menu item when All Address Books is selected, maybe produce a popup saying to choose a specific address book. I don't like the idea of hiding the Export menu item in a context menu, as that would be hurt its discoverability.

Comment 8

[:aceman]

Attachment: disable export

I'm not sure we can do anything proper this late before release. So this is a quick fix to disable the Export menuitem.

For the proper fix, this would need proper specification on how it should behave:
1. somebody pointed out possible duplicates in multiple ABs. We do not have infrastructure to merge duplicates yet. It seems to me ABs are just dumped out in the requested format.
2. LDAP addressbooks do not show any contacts if there is no search term entered. Even when there is, in my experiments the exported file was still empty (only the headers).

Comment 9

[Andrey Utkin]

> I'm not sure we can do anything proper this late before release. So this is a quick fix to disable the Export menuitem.

Very surprising approach. However, following it, dev team would reach great success by denying users to download Thunderbird. No bugs exposed at all!


> For the proper fix, this would need proper specification on how it should behave

I believe some clue could be found by looking how it was supposed to behave initially (e.g. at the moment of introduction of this feature).

I don't care about duplicates and I don't use LDAP. Still every kind of exporting procedure ends up with empty file.

Comment 10

[:aceman]

(In reply to Andrey Utkin from comment #9)
> I believe some clue could be found by looking how it was supposed to behave
> initially (e.g. at the moment of introduction of this feature).

A feature to export all ABs in one go was never introduced.

> I don't care about duplicates and I don't use LDAP. Still every kind of
> exporting procedure ends up with empty file.

You may not care about dupes, but most people will, as even the contacts found in Personal Addressbook will probably be found in the Collected Addresses (if we automatically export that one too). Can you say you are not in that state?
Importing a file with dupes may cause problems.

Comment 11

[Magnus Melin [:mkmelin]]

(In reply to :aceman from comment #8)
> Created attachment 8711505 [details] [diff] [review]

How about we loop through address books and export each separately with the dialog popping up to have you select filename for each? And maybe break if the user ever chooses cancel.

Comment 12

[:aceman]

Yes, that would be easiest (as it seems our atomic code unit is like that now = ask for file name, export one AB). But the user experience seems strange to me. Also, there is still the problem whether to export LDAP ABs, which do not seem to work.

Comment 13

[Magnus Melin [:mkmelin]]

I'd still prefer that, as a totally disabled export is hard to understand. Let's just skip LDAP abs.

Comment 14

[:aceman]

But there is no indication in the target file picker about which AB is going to be exported. Maybe we can show it in the titlebar?

Comment 15

[Magnus Melin [:mkmelin]]

Sounds good to me.

Comment 16

[:aceman]

Attachment: export all ABs individually

OK, this should do it.

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 8713805 [details] [diff] [review]
export all ABs individually

Review of attachment 8713805 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/addrbook/content/addressbook.js
@@ +437,5 @@
> +  let directories = MailServices.ab.directories;
> +
> +  while (directories.hasMoreElements()) {
> +    let directory = directories.getNext();
> +    // Do not export LDAP ABs and the Collected addresses AB.

Why not the collected ab? I think that's unexpected

::: mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties
@@ +129,5 @@
>  abReplicationOfflineWarning=You must be online to perform LDAP replication.
>  abReplicationSaveSettings=Settings must be saved before a directory may be downloaded.
>  
>  # For importing / exporting
> +ExportAddressBookNameTitle=Export Address Book "%S"

I'd prefer it as Export Address Book - %S

Also, please add a l10n note that the string to fill in is the address book name

Comment 18

[:aceman]

Attachment: export all ABs individually v1.1

OK, done.

Comment 19

[Magnus Melin [:mkmelin]]

Comment on attachment 8714124 [details] [diff] [review]
export all ABs individually v1.1

Review of attachment 8714124 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good thx! r=mkmelin

Would be nice if it suggested file names too, but that would be for another bug.

Comment 20

[:aceman]

Thanks.

Comment 21

[Ian Neal]

Wouldn't SM also need the JS/xul changes?

Comment 22

[:aceman]

Not necessarily. I fixed the needed string to not break SM. But I did not port the code in case you don't want it. I'm sure I wanted to ask about it but somehow I can't see it in this bug :(

Comment 23

[:aceman]

So please tell me if you file a "porting" bug or I should make a SM version patch here.

Comment 24

[:aceman]

https://hg.mozilla.org/comm-central/rev/27b50a06b9b2

Comment 25

[Philip Chee]

(In reply to :aceman from comment #23)
> So please tell me if you file a "porting" bug or I should make a SM version
> patch here.
I filed Bug 1246897 for SeaMonkey so that we can set the right tracking flags. I also set the assignee field to you. I hope this is acceptable.

Comment 26

[:aceman]

(In reply to Philip Chee from comment #25)
> I filed Bug 1246897 for SeaMonkey so that we can set the right tracking
> flags. I also set the assignee field to you. I hope this is acceptable.
Sure, if it is acceptable to you that I just copy the code to the relevant SM files, but let the reviewer test it :)

Comment 27

[Jorg K (CEST = GMT+2)]

To me, this bug looks resolved. Since this has tracking-thunderbird45+, I will request uplift.
Please let me know if I'm wrong.

Comment 28

[Jorg K (CEST = GMT+2)]

Oh, there are string changes, so perhaps too late for uplift.

Comment 29

[Kent James (:rkent)]

We discuss doing a TB45 port that could take the TB 45 string and simple append the title to it.

Comment 30

[:aceman]

(In reply to Jorg K (GMT+1) from comment #27)
> To me, this bug looks resolved. Since this has tracking-thunderbird45+, I
> will request uplift.
> Please let me know if I'm wrong.

I just kept it open until the SM port is decided. It was split in a new bug so this one is resolved.

Comment 31

[Jorg K (CEST = GMT+2)]

Aceman, you didn't answer the other issue: Will you prepare a patch for TB45 without the string change, so you use the old string and add your %S in the code. This is what's needed to land on beta and we can test this on Aurora, too.

Comment 32

[:aceman]

Yes, I'll do.

Comment 33

[:aceman]

Attachment: patch - version for TB45

I've developed this on trunk, please check if it applies to 45 branch.

Comment 34

[Kent James (:rkent)]

Yes it applies.

Comment 35

[:aceman]

Thanks. Then please review and push to branch when appropriate.

Comment 36

[Kent James (:rkent)]

Comment on attachment 8718010 [details] [diff] [review]
patch - version for TB45

Review of attachment 8718010 [details] [diff] [review]:
-----------------------------------------------------------------

I reviewed the changes, also tested on a TB45 build and it works fine.

Comment 37

[Jorg K (CEST = GMT+2)]

Which patch would you like to land on Aurora (TB 46)? The TB45 version or the TB47 version?

Comment 38

[Kent James (:rkent)]

(In reply to Jorg K (GMT+1) from comment #37)
> Which patch would you like to land on Aurora (TB 46)? The TB45 version or
> the TB47 version?
My understanding is that aurora by default has a string freeze, so I would land the tb45 version. Generally you should be checking for string changes as part of the aurora approval process. In the long run that might be something we should reconsider.

Comment 39

[Jorg K (CEST = GMT+2)]

Comment on attachment 8718010 [details] [diff] [review]
patch - version for TB45

[Approval Request Comment]
Regression caused by (bug #): No regression.
User impact if declined: Puzzling behaviour, nothing gets exported.
Testing completed (on c-c, etc.): Manual testing by Kent on TB 45 version, see comment #36.
Risk to taking this patch (and alternatives if risky):
Small change, doesn't appear risky.

Comment 40

[Jorg K (CEST = GMT+2)]

Aurora (TB 46) - pushed the TB 45 version:
https://hg.mozilla.org/releases/comm-aurora/rev/626a4c5c5c88
(DONTBUILD, Daily build will pick it up)

Comment 41

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #40)
> (DONTBUILD, Daily build will pick it up)
I was wrong, it doesn't get picked up. Oh well, it will be picked up with the next push. Sorry.

Comment 42

[Kent James (:rkent)]

Comment on attachment 8718010 [details] [diff] [review]
patch - version for TB45

http://hg.mozilla.org/releases/comm-beta/rev/e69c46711b1a

Comment 44

[David E. Ross]

Windows 7 Ultimate
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

This is still a problem.

Comment 45

[Jorg K (CEST = GMT+2)]

This was first landed on TB 47 and then uplifted to TB 45 and TB 46 in February 2016. So this bug is well and truly done.

Please file a new bug.

You can only reopen a bug under two circumstances:
1) The patch was backed out or after landing it becomes obviously immediately
   that a follow-up is necessary which can be done within a few days in the same bug.
2) A bug was closed without a fix and you're not happy with the decision.

Neither case applies here.

Comment 46

[Thomas D. (:thomas8)]

(In reply to David E. Ross from comment #44)
> Windows 7 Ultimate
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101
> Thunderbird/45.5.1
> 
> This is still a problem.

David, thanks for reporting your issue with exporting "All Address Books", which we appreciate. Unfortunately, Jörg is right that we can't handle this here on the old bug, and it wouldn't make much sense even if we could. Could you please open a new bug with steps to reproduce, actual result, and expected result. The easiest way is to use the "Clone this bug" link at the bottom of this bug:

https://bugzilla.mozilla.org/enter_bug.cgi?format=__default__&cloned_bug_id=1182460

Pls include information about the type of ABs which you are exporting, and any other special settings, unusual data, or circumstances including addons (pls test in TB-safe-mode). Tia.

Comment 47

[Charles]

(In reply to David E. Ross from comment #44)
> Windows 7 Ultimate
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101
> Thunderbird/45.5.1
> 
> This is still a problem.

Must be something specific to your profile, becasue it works for me, although my quick test was the first time I tried it, and I would never use it for one reason:

it doesn't default to the Address Book name for the name of the exported file.

The only reason I can think of for this is it must be because it is difficult or impossible to get the AB name during the export routine?

Comment 48

[David E. Ross]

Never mind.  The problem was that a separate file-navigation window pops up for each address book.  After the first such window, I kept trying to dismiss without filling in a file-name.  I should have noticed that addressbook name in the title bars of the file-navigation windows.

Comment 49

[Charles]

(In reply to David E. Ross from comment #48)
> I should have noticed that addressbook name in the title bars of the file-navigation windows.

Aargh, I missed that.

So, why not just use the Address Book Name for the name of the exported file and lose all of the prompts (I have 8 Address Books, so that is a lot of prompts to have to deal with).