Closed Bug 536542 Opened 10 years ago Closed 10 years ago

In all header view mode show all email addresses of To/CC fields

Categories

(Thunderbird :: Message Reader UI, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a1

People

(Reporter: joachim.herb, Assigned: joachim.herb)

References

Details

Attachments

(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091209 Shredder/3.1a1pre ID:20091209003527

At the moment only the first two addresses of the to/cc fields are shown in both normal and all header view mode.

At different place it has been suggested that in all header view mode all email addresses of the to/cc fields are shown.
https://addons.mozilla.org/thunderbird/reviews/display/13564
http://getsatisfaction.com/mozilla_messaging/topics/show_all_headers_should_show_all_addressees_not_a_more_button

It would be nice if all addresses are shown in full header mode either as default or by adding a new option.

Reproducible: Always
OS: Windows 7 → All
Hardware: x86 → All
Attachment #418992 - Flags: review?(dmose)
Flags: blocking-thunderbird3.1?
Confirming RFE. Now that bug 530239 is visible in the 3.0.1 release, it's even more evident that some better solution is needed. I'd see this proposal as an extension to bug 456596, which may mitigate the issue by allowing to display more addresses by default before the list is truncated.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
Depends on: 456596
Comment on attachment 418992 [details] [diff] [review]
First patch to show all addresses in full header view mode

The UI you propose makes sense to me; requesting ui-review so that clarkbw make a call here (note that he's on vacation this coming week).  Assuming it does get ui-review, would you be willing to write a MozMill automated test for this?
Attachment #418992 - Flags: ui-review?
Attachment #418992 - Flags: ui-review? → ui-review?(clarkbw)
I don't _think_ we'd actually hold the release for this if it were the last bug standing, so marking as blocking-.  It'd be good to get, however, so marking as wanted+.
Flags: blocking-thunderbird3.1? → wanted-thunderbird-
Comment on attachment 418992 [details] [diff] [review]
First patch to show all addresses in full header view mode

this makes sense, though it could have issues of making the all headers mode useless with large to or cc lists.
Attachment #418992 - Flags: ui-review?(clarkbw) → ui-review+
Assignee: nobody → herb
Herb, would you be willing to code up a few simple mozmill tests here?
(I'd expect it to be straightforward to add them to test-message-header.js).
I will try. Unfortunately I will get to write some tests earliest next week(end).
I am stuck at the very beginning: I cannot run the file test-message-header.js from the mozmill add-on. It always stops with the following error message:

test :: Started running test: test_that_msg_without_date_clears_previous_headers
test :: Started running test: test_a11y_attrs
test :: Started running test: test_add_tag_with_really_long_label
fail :: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]
message: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]
QueryInterface: function QueryInterface() { [native code] }
result: 2152857618
name: NS_ERROR_FILE_NOT_FOUND
filename: file:///C:/Users/herb/AppData/Roaming/Thunderbird/Profiles/bbknerdc.default/extensions/mozmill@mozilla.com/resource/modules/frame.js -> file:///C:/Users/herb/workspace/comm-central/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///C:/Users/herb/workspace/comm-central/mailnews/test/resources/logHelper.js
lineNumber: 12
columnNumber: 0
location: JS frame :: file:///C:/Users/herb/AppData/Roaming/Thunderbird/Profiles/bbknerdc.default/extensions/mozmill@mozilla.com/resource/modules/frame.js -> file:///C:/Users/herb/workspace/comm-central/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///C:/Users/herb/workspace/comm-central/mailnews/test/resources/logHelper.js :: :: line 12
inner: null
data: null
initialize: function initialize() { [native code] }
test: function setupModule(module) { var fdh = collector.getModule("folder-display-helpers"); fdh.installInto(module); var wh = collector.getModule("window-helpers"); wh.installInto(module); folder = create_folder("MessageWindowA"); var msg = create_message({cc: [["John Doe", "john.doe@momo.invalid"]], clobberHeaders: {Newsgroups: "alt.test", 'Reply-To': "J. Doe ", 'Content-Base': "http://example.com/", Bcc: "Richard Roe "}}); add_message_to_folder(folder, msg); }
test :: Started running test: setupModule

Also other tests fail with this error message (NS_ERROR_FILE_NOT_FOUND), e.g. the attachment test, folder-pane test.

I am probably missing something. Any help?

I think the test I should write will look something like this:
Create message with 3 or more To and CC addresses.
Turn on "all header mode". Check if all addresses are visible. Turn it off check that only one address plus the more button is shown. Create message with one recipient. Check if only this (one) address and no more button is shown.
Herb, that looks like it is missing the IOUtils.js file - see line 12 of logHelper.js - which is part of the build.

Are you trying to run the latest comm-central test files against Thunderbird 3.0? If so, I don't think that will work and you'll need to run against the latest Thunderbird 3.1 builds.
You are right, I tried it with 3.0.3 and got the error message of comment #9.

But with a freshly build comm-central (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a4pre) Gecko/20100328 Shredder/3.2a1pre ID:20100328232215), beta1 of 3.1 (Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.2.2pre) Gecko/20100302 Lanikai/3.1b1 ID:20100302141414) and the latest nightly build (Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2pre) Gecko/20100330 Lanikai/3.1b2pre ID:20100330031754) I get another error message. I started thunderbird with a fresh profile.

fail :: Component returned failure code: 0x80550013 [nsIMsgFolder.addSubfolder]
message: Component returned failure code: 0x80550013 [nsIMsgFolder.addSubfolder]
QueryInterface: function QueryInterface() { [native code] }
result: 2153054227
name: null
filename: file:///C:/Users/herb/AppData/Roaming/Thunderbird/Profiles/qtno9xwb.mozmill/extensions/mozmill@mozilla.com/resource/modules/frame.js -> file:///C:/Users/herb/workspace/comm-central/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///C:/Users/herb/workspace/comm-central/mailnews/test/resources/messageInjection.js
lineNumber: 118
columnNumber: 0
location: JS frame :: file:///C:/Users/herb/AppData/Roaming/Thunderbird/Profiles/qtno9xwb.mozmill/extensions/mozmill@mozilla.com/resource/modules/frame.js -> file:///C:/Users/herb/workspace/comm-central/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///C:/Users/herb/workspace/comm-central/mailnews/test/resources/messageInjection.js :: configure_message_injection :: line 118
inner: null
data: null
initialize: function initialize() { [native code] }
test: function setupModule(module) { var fdh = collector.getModule("folder-display-helpers"); fdh.installInto(module); var wh = collector.getModule("window-helpers"); wh.installInto(module); folder = create_folder("MessageWindowA"); var msg = create_message({cc: [["John Doe", "john.doe@momo.invalid"]], clobberHeaders: {Newsgroups: "alt.test", 'Reply-To': "J. Doe ", 'Content-Base': "http://example.com/", Bcc: "Richard Roe "}}); add_message_to_folder(folder, msg); }
Sid can you give herb a hand ?
I got mozmill to run (the problem was, that I have to start each (and every) time with a fresh profile). 

Now the "only" problem is that I have to get a little bit more experience with mozmill. 

Is there a chance to get this bug landed for 3.1 anymore? 

If yes should I post my not yet working mozmill testcase here or where else could I get help?
Given that we're trying to feature freeze in 6 hours, I don't think it's super likely, given that it would depend on one of us finding time to help you out with the mozmillery as well as reviewing between now and then, but it's not technically impossible.

That said, I think posting your mozmill work-in-progress patch here for help/comment is a fine strategy in any case, so I'd very much recommend that.
What I am basically trying to achieve is check if there is a more button in the header and click on it. This is how I think it should work (inside the file test-message-header.js in a new test case):

var toMoreButton = mc.aid("expandedtoBox", {class: "moreIndicator"});
controller.assertNode(toMoreButton); 
controller.click(toMoreButton);

This it not working as the more button is not found.

If I use the following construction, the more button is found:
var toMoreButton = new elementslib.Lookup(controller.window.document, 
    '/id("messengerWindow")/id("tabmail-container")/id("tabmail")/id("tabpanelcontainer")' + 
    '/id("mailContent")/id("messengerBox")/[2]/id("messagesBox")/id("messagepanebox")' +
    '/id("singlemessage")/id("msgHeaderView")/id("msgHeaderViewDeck")/id("expandedHeaderView")' +
    '/id("expandedHeadersBox")/id("expandedHeadersBottomBox")/id("expandedHeaders2")/' +
    'id("expandedHeader2Rows")/id("expandedtoRow")/id("expandedtoBox")/' + 
    'anon({"anonid":"longEmailAddresses"})/anon({"anonid":"emailAddresses"})' + 
    '/anon({"anonid":"more"})/anon({"class":"moreIndicator","value":"more","onclick":' + 
    '"this.parentNode.parentNode.parentNode.parentNode.toggleWrap()"})');

But only if the more button is shown for the first time. If I click on the button, then view another message and then again the first one (which has a more button) the second version is not working anymore.

Also I tried these variants:
var toMoreButton = mc.aid("expandedtoBox", {value: "more"});
var toMoreButton = mc.eid("expandedtoBox", {tagName: "label"});

var exp1 = mc.e("expandedtoBox");
var node = controller.window.document.getAnonymousElementByAttribute(exp1, "class", "moreIdicator");

But all of them result in undefined/null values.

Any ideas?
herb, look into the current implementation of mailWidgets.xml on comm-central. The code has changed substantially since bug 550487 was checked in, with minor updates by bug 565209. This should simplify the task in "buildViews" though by just having to pass a "true" as the 2nd parameter of this.fillAddressesNode()
to fill all addresses if View > Headers > All is selected.
Should a new patch be based on comm-central or comm-1.9.2? Or doesn't it matter?
The code on comm-central is more recent (thus different), and with 3.1 almost out, any patch will need to go on trunk first before it is ok for comm-1.9.2.

> https://wiki.mozilla.org/Thunderbird/Thunderbird3_Security_And_Stability_Releases
(still needs to be updated for 3.1, but for now where it says 3.0, read 3.1).
Patch based on suggestion in  Comment #16
Includes mozmill test case.
Attachment #418992 - Attachment is obsolete: true
Attachment #451153 - Flags: review?(dmose)
Attachment #418992 - Flags: review?(dmose)
Two ad-hoc nits I've noticed:

>              if (this.maxLinesBeforeMore > 1)

You can move this up into the else{} branch, otherwise the "singleline" attribute is removed twice.

> +function test_show_all_heaeder_line_mode() {

Function has "heaeder" rather than "header" in its name.
Attached patch Cleanup (obsolete) — Splinter Review
Attachment #451153 - Attachment is obsolete: true
Attachment #451303 - Flags: review?(dmose)
Attachment #451153 - Flags: review?(dmose)
Attachment #451303 - Attachment is patch: true
Attachment #451303 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 451303 [details] [diff] [review]
Cleanup

My week's pretty heavily scheduled; I'm going to hand this review off to Blake in the hopes that he can get to it more quickly.
Attachment #451303 - Flags: review?(dmose) → review?(bwinton)
Comment on attachment 451303 [details] [diff] [review]
Cleanup

>+++ b/mail/base/content/mailWidgets.xml
>@@ -449,12 +449,19 @@
>+            const dt = Components.interfaces.nsMimeHeaderDisplayTypes;
>+            var headerchoice = Application.prefs.getValue("mail.show_headers", 

Trailing space on this line.

>+                                                          dt.AllHeaders);
>+            if (headerchoice == dt.AllHeaders) {
>+              this.fillAddressesNode(this.emailAddresses, true);
>               this.longEmailAddresses.removeAttribute("singleline");
>+            }
>+            else {
>+              this.fillAddressesNode(this.emailAddresses, false);
>+              // force a single line only in the default n=1 case
>+              if (this.maxLinesBeforeMore > 1)
>+                this.longEmailAddresses.removeAttribute("singleline");
>+            } 

And on this line.

>+++ b/mail/test/mozmill/message-header/test-message-header.js
>@@ -317,6 +317,46 @@
>+function change_to_header_normal_mode() {
>+  mc.click(new elementslib.ID(mc.window.document, "menu_View"));
>+  mc.click(new elementslib.ID(mc.window.document, "viewnormalheaders"));

We should be able to do this a little easier, with something along the lines of 
mc.click(mc.menus.View.viewnormalheaders);

>+function change_to_all_header_mode() {
>+  mc.click(new elementslib.ID(mc.window.document, "menu_View"));
>+  mc.click(new elementslib.ID(mc.window.document, "viewallheaders"));

Ditto this.

>@@ -407,6 +447,28 @@
> /**
>+ * Test that changing to all header lines mode displays all the addresses.
>+ * @param toDescription the description node for the "to" field
>+ */
>+function subtest_change_to_all_header_mode(toDescription) {

Since we only use this in one place, we should just inline the code.
But maybe not.

r=me with those nits fixed.

Later,
Blake.
Attachment #451303 - Flags: review?(bwinton) → review+
(In reply to comment #23)
> >+++ b/mail/test/mozmill/message-header/test-message-header.js
> >@@ -317,6 +317,46 @@
> >+function change_to_header_normal_mode() {
> >+  mc.click(new elementslib.ID(mc.window.document, "menu_View"));
> >+  mc.click(new elementslib.ID(mc.window.document, "viewnormalheaders"));
> 
> We should be able to do this a little easier, with something along the lines of 
> mc.click(mc.menus.View.viewnormalheaders);

This line is enough:
mc.click(new elementslib.ID(mc.window.document, "viewnormalheaders"));

> >@@ -407,6 +447,28 @@
> > /**
> >+ * Test that changing to all header lines mode displays all the addresses.
> >+ * @param toDescription the description node for the "to" field
> >+ */
> >+function subtest_change_to_all_header_mode(toDescription) {
> 
> Since we only use this in one place, we should just inline the code.
> But maybe not.
> 
> r=me with those nits fixed.
How to proceed? Should I create another patch or will be the current patch checked in?
It means that you should post a new patch with Blake's comments addressed,
then you can carry forward the ui-r+ from attachment 418992 [details] [diff] [review] and the
r+ from attachment 451303 [details] [diff] [review] (just mark ui-review+ and review+ when
you post a new version yourself).

> (In reply to comment #23)
> > We should be able to do this a little easier, with something along the lines of 
> > mc.click(mc.menus.View.viewnormalheaders);
> 
> This line is enough:
> mc.click(new elementslib.ID(mc.window.document, "viewnormalheaders"));

I'd understand Blake's request that you can simplify your current version by using the proposed construct, thus reducing code complexity here.
Attachment #451303 - Attachment is obsolete: true
Attachment #452552 - Flags: ui-review+
Attachment #452552 - Flags: review+
I'm setting the checkin-needed keyword for you, someone should stop by within the next few days and push it to comm-central. I don't think this would qualify for 3.1 based on Dan's comment #14, but you can set approval-thunderbird3.1.1? if you want to try (after it has checked in and the tinderboxes are happy).
Depends on: 565209
No longer depends on: 456596
Keywords: checkin-needed
Whiteboard: [c-n: comm-central]
Checked in: http://hg.mozilla.org/comm-central/rev/dc24b1578cc4
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: comm-central]
Target Milestone: --- → Thunderbird 3.2a1
Depends on: 576611
You need to log in before you can comment on or make changes to this bug.