Closed
Bug 1182460
Opened 9 years ago
Closed 7 years ago
Exporting/saving address book with "All address books" selected exports only headers, or 0 byte file
Categories
(Thunderbird :: Address Book, defect)
Tracking
(thunderbird45+ fixed, thunderbird46 fixed, thunderbird47 fixed)
RESOLVED
FIXED
Thunderbird 47.0
People
(Reporter: anjeyelf, Assigned: aceman)
References
Details
Attachments
(2 files, 2 obsolete files)
6.81 KB,
patch
|
mkmelin
:
review+
philip.chee
:
review+
|
Details | Diff | Splinter Review |
4.47 KB,
patch
|
rkent
:
review+
jorgk-bmo
:
approval-comm-aurora+
rkent
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
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.
Updated•9 years ago
|
Component: Untriaged → Address Book
Summary: 'All Address Book' - Export → export address book with "All address books" selected exports only headers
Comment 2•9 years ago
|
||
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.
Comment 4•9 years ago
|
||
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.
Updated•9 years ago
|
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
Updated•9 years ago
|
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
Updated•8 years ago
|
tracking-thunderbird45:
--- → +
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)
Comment 9•8 years ago
|
||
> 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.
Assignee | ||
Comment 10•8 years ago
|
||
(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•8 years ago
|
||
(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.
Assignee | ||
Comment 12•8 years ago
|
||
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•8 years ago
|
||
I'd still prefer that, as a totally disabled export is hard to understand. Let's just skip LDAP abs.
Updated•8 years ago
|
Attachment #8711505 -
Flags: review?(mkmelin+mozilla) → review-
Assignee | ||
Comment 14•8 years ago
|
||
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•8 years ago
|
||
Sounds good to me.
Assignee | ||
Comment 16•8 years ago
|
||
OK, this should do it.
Attachment #8711505 -
Attachment is obsolete: true
Attachment #8713805 -
Flags: review?(mkmelin+mozilla)
Comment 17•8 years ago
|
||
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)
Assignee | ||
Comment 18•8 years ago
|
||
OK, done.
Attachment #8713805 -
Attachment is obsolete: true
Attachment #8714124 -
Flags: review?(mkmelin+mozilla)
Comment 19•8 years ago
|
||
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)
Updated•8 years ago
|
Attachment #8714124 -
Flags: review?(philip.chee) → review+
Assignee | ||
Comment 20•8 years ago
|
||
Thanks.
Attachment #8714124 -
Flags: review?(iann_bugzilla)
Assignee | ||
Comment 22•8 years ago
|
||
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)
Assignee | ||
Comment 23•8 years ago
|
||
So please tell me if you file a "porting" bug or I should make a SM version patch here.
Assignee | ||
Comment 24•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/27b50a06b9b2
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 47.0
Comment 25•8 years ago
|
||
(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.
Assignee | ||
Comment 26•8 years ago
|
||
(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•8 years ago
|
||
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: 8 years ago
status-thunderbird45:
--- → affected
status-thunderbird46:
--- → affected
status-thunderbird47:
--- → fixed
Flags: needinfo?(acelists)
Resolution: --- → FIXED
Comment 28•8 years ago
|
||
Oh, there are string changes, so perhaps too late for uplift.
Comment 29•8 years ago
|
||
We discuss doing a TB45 port that could take the TB 45 string and simple append the title to it.
Assignee | ||
Comment 30•8 years ago
|
||
(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)
Comment 31•8 years ago
|
||
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)
Assignee | ||
Comment 33•8 years ago
|
||
I've developed this on trunk, please check if it applies to 45 branch.
Attachment #8718010 -
Flags: review?(rkent)
Comment 34•8 years ago
|
||
Yes it applies.
Assignee | ||
Comment 35•8 years ago
|
||
Thanks. Then please review and push to branch when appropriate.
Comment 36•8 years ago
|
||
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+
Comment 37•8 years ago
|
||
Which patch would you like to land on Aurora (TB 46)? The TB45 version or the TB47 version?
Flags: needinfo?(rkent)
Comment 38•8 years ago
|
||
(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 39•8 years ago
|
||
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+
Comment 40•8 years ago
|
||
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•8 years ago
|
||
(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•8 years ago
|
||
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+
Updated•8 years ago
|
Comment 44•7 years ago
|
||
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 → ---
Comment 45•7 years ago
|
||
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: 8 years ago → 7 years ago
Resolution: --- → FIXED
Comment 46•7 years ago
|
||
(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)
Comment 47•7 years ago
|
||
(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•7 years ago
|
||
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)
Comment 49•7 years ago
|
||
(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.
Description
•