1659162 - Error when creating mailing list from contacts sidebar context: Uncaught ReferenceError: gDirectoryTreeView is not defined

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Thomas D. (:thomas8)]

Seen on daily 81.0a1 (2020-08-14) (64-bit)

Str:

1. compose new msg
2. from contacts sidebar, get ab context menu (mini-burger, or right-click pane whitespace)
3. New List

Actual:

daily 81.0a1 (2020-08-14) (64-bit) and
beta 80.0b3 (32-bit):

Uncaught ReferenceError: gDirectoryTreeView is not defined
goNewListDialog chrome://messenger/content/addressbook/abCommon.js:788
AbPanelNewList chrome://messenger/content/addressbook/abContactsPanel.js:199
oncommand chrome://messenger/content/addressbook/abContactsPanel.xhtml:1
abCommon.js:788:5

Release 78.1.1 (64-bit):

[Exception... "JavaScript component does not have a method named: "hasEmailAddress"'JavaScript component does not have a method named: "hasEmailAddress"' when calling method: [nsIAbCard::hasEmailAddress]" nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)" location: "JS frame :: chrome://messenger/content/msgHdrView.js :: UpdateExtraAddressProcessing :: line 1548" data: no] 2 msgHdrView.js:1548:17

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1659162_sidebar_gDirectoryTreeView.patch

Seems like a harmless error.
But we should fix it. The sidebar didn't have the expected include. However, trying to select the list after it's creation would be weird in the sidebar, where you can search by name and then the new list would all the sudden get selected.

Comment 2

[Thomas D. (:thomas8)]

Comment on attachment 9170916 [details] [diff] [review]
bug1659162_sidebar_gDirectoryTreeView.patch

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

Thanks. This fixes it. However, do we need to load abTrees.js into contacts sidebar just to check for gDirectoryTreeView which we already know does *not* exist there?
Commit message: capitalization in r=ThomasD
I'll upload a patch just now with these nits fixed.

::: mail/components/addrbook/content/abCommon.js
@@ +783,5 @@
>      "",
>      "chrome,modal,resizable=no,centerscreen",
>      params
>    );
> +  if (params.newListURI && gDirectoryTreeView.selection) {

Check for existence of gDirectoryTreeView while avoiding the reference error.

```
  if (params.newListURI && typeof gDirectoryTreeView !== "undefined") {
```

::: mail/components/addrbook/content/abContactsPanel.xhtml
@@ +32,5 @@
>    <script src="chrome://messenger/content/addressbook/abResultsPane.js"/>
>    <script src="chrome://messenger/content/addressbook/abContactsPanel.js"/>
>    <script src="chrome://messenger/content/jsTreeView.js"/>
>    <script src="chrome://messenger/content/addressbook/abView.js"/>
> +  <script src="chrome://messenger/content/addressbook/abTrees.js"/>

Now we don't need to load this into memory any more.

Comment 3

[Thomas D. (:thomas8)]

Attachment: 1659162_sidebar_gDirectoryTreeView.patch

New patch with the proposed little amendments.

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to Thomas D. (:thomas8) from comment #2)

Thanks. This fixes it. However, do we need to load abTrees.js into contacts
sidebar just to check for gDirectoryTreeView which we already know does
not exist there?

Strictly speaking it's likely not needed. However, since the files are split up in these weird ways, I think including it is the correct way - otherwise there's a trap for future devs who do not realize that's not there. The linting is also set up not to help, since it says "you can assume abTreees.js is loaded here". https://searchfox.org/comm-central/rev/c25f688917f090fb02d31f0160e484fd2b8ad93e/mail/components/addrbook/content/abCommon.js#9
That it's not really common code but still in abCommon.js is perhaps the real bug, but I didn't want to go down that rabbit hole...

Comment 5

[Thomas D. (:thomas8)]

Attachment: 1659162_sidebar_gDirectoryTreeView.patch

(In reply to Magnus Melin [:mkmelin] from comment #4)

(In reply to Thomas D. (:thomas8) from comment #2)

However, do we need to load abTrees.js into contacts sidebar
Strictly speaking it's likely not needed. However, since the files are split up in these weird ways, I think including it is the correct way

Yeah, ok, sort of. Thanks for the explanation!
Patch adjusted per your preference.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/315dd887eb1e
Don't try to select a new mailing list if it was created from Contacts Sidebar. r=ThomasD

Comment 7

[Magnus Melin [:mkmelin]]

This is harmless so we don't need to uplift it anywhere.