Closed Bug 1182460 Opened 4 years ago Closed 3 years ago

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

Categories

(Thunderbird :: Address Book, defect)

38 Branch
defect
Not set

Tracking

(thunderbird45+ fixed, thunderbird46 fixed, thunderbird47 fixed)

RESOLVED FIXED
Thunderbird 47.0
Tracking Status
thunderbird45 + fixed
thunderbird46 --- fixed
thunderbird47 --- fixed

People

(Reporter: anjeyelf, Assigned: aceman)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
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.
Component: Untriaged → Address Book
Summary: 'All Address Book' - Export → export address book with "All address books" selected exports only headers
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.
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.
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.
Duplicate of this bug: 1222681
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: export address book with "All address books" selected exports only headers → Exporting/saving address book with "All address books" selected exports only headers, or 0 byte file for LDIF
Summary: Exporting/saving address book with "All address books" selected exports only headers, or 0 byte file for LDIF → Exporting/saving address book with "All address books" selected exports only headers, or 0 byte file
Duplicate of this bug: 1226902
Duplicate of this bug: 1241950
Attached patch disable export (obsolete) — Splinter Review
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).
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #8711505 - Flags: review?(mkmelin+mozilla)
> 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.
(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.
(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.
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.
I'd still prefer that, as a totally disabled export is hard to understand. Let's just skip LDAP abs.
Attachment #8711505 - Flags: review?(mkmelin+mozilla) → review-
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?
Sounds good to me.
Attached patch export all ABs individually (obsolete) — Splinter Review
OK, this should do it.
Attachment #8711505 - Attachment is obsolete: true
Attachment #8713805 - Flags: review?(mkmelin+mozilla)
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
Attachment #8713805 - Flags: review?(mkmelin+mozilla)
OK, done.
Attachment #8713805 - Attachment is obsolete: true
Attachment #8714124 - Flags: review?(mkmelin+mozilla)
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.
Attachment #8714124 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8714124 - Flags: review?(iann_bugzilla)
Attachment #8714124 - Flags: review?(philip.chee)
Attachment #8714124 - Flags: review?(philip.chee) → review+
Thanks.
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Attachment #8714124 - Flags: review?(iann_bugzilla)
Wouldn't SM also need the JS/xul changes?
Flags: needinfo?(acelists)
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 :(
Flags: needinfo?(acelists)
So please tell me if you file a "porting" bug or I should make a SM version patch here.
https://hg.mozilla.org/comm-central/rev/27b50a06b9b2
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 47.0
Blocks: 1246897
(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.
(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 :)
To me, this bug looks resolved. Since this has tracking-thunderbird45+, I will request uplift.
Please let me know if I'm wrong.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: needinfo?(acelists)
Resolution: --- → FIXED
Oh, there are string changes, so perhaps too late for uplift.
We discuss doing a TB45 port that could take the TB 45 string and simple append the title to it.
(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.
Flags: needinfo?(acelists)
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.
Flags: needinfo?(acelists)
Yes, I'll do.
Flags: needinfo?(acelists)
I've developed this on trunk, please check if it applies to 45 branch.
Attachment #8718010 - Flags: review?(rkent)
Yes it applies.
Thanks. Then please review and push to branch when appropriate.
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.
Attachment #8718010 - Flags: review?(rkent) → review+
Which patch would you like to land on Aurora (TB 46)? The TB45 version or the TB47 version?
Flags: needinfo?(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.
Flags: needinfo?(rkent)
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.
Attachment #8718010 - Flags: approval-comm-beta?
Attachment #8718010 - Flags: approval-comm-aurora+
Aurora (TB 46) - pushed the TB 45 version:
https://hg.mozilla.org/releases/comm-aurora/rev/626a4c5c5c88
(DONTBUILD, Daily build will pick it up)
(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 on attachment 8718010 [details] [diff] [review]
patch - version for TB45

http://hg.mozilla.org/releases/comm-beta/rev/e69c46711b1a
Attachment #8718010 - Flags: approval-comm-beta? → approval-comm-beta+
Duplicate of this bug: 1246703
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 4 years ago3 years ago
Resolution: --- → FIXED
(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.
Flags: needinfo?(david)
(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?
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.
Flags: needinfo?(david)
(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).
You need to log in before you can comment on or make changes to this bug.