The default bug view has changed. See this FAQ.

Errors when changing in prefs the "Show only display name for people in my address book"

RESOLVED FIXED in Thunderbird 41.0

Status

Thunderbird
Mail Window Front End
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Paenglab, Assigned: aceman)

Tracking

Trunk
Thunderbird 41.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.94 KB, patch
squib
: review+
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
When I disable the pref "Show only display name for people in my address book" I get this error:

Error: TypeError: threadTree.treeBoxObject.invalid is not a function
Source file: chrome://messenger/content/msgMail3PaneWindow.js
Line: 365

and on enabling

Error: TypeError: gFolderDisplay.view.dbView is null
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1244

When a message is selected also:

Error: TypeError: imgs is undefined
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 3297
(Assignee)

Comment 1

2 years ago
(In reply to Richard Marti (:Paenglab) from comment #0)
> When I disable the pref "Show only display name for people in my address
> book" I get this error:
> 
> Error: TypeError: threadTree.treeBoxObject.invalid is not a function
> Source file: chrome://messenger/content/msgMail3PaneWindow.js
> Line: 365

It should be .invalidate.

> Error: TypeError: gFolderDisplay.view.dbView is null
> Source File: chrome://messenger/content/msgMail3PaneWindow.js
> Line: 1244

If there is no message selected (e.g. we show the account central), there is no .dbView and nothing to reload.

> When a message is selected also:
> 
> Error: TypeError: imgs is undefined
> Source File: chrome://messenger/content/mailWindowOverlay.js
> Line: 3297
This one seems to only happen when the Preferences are in a tab.

The offending code is:
  let browser = getBrowser();
  let doc = browser && browser.contentDocument ? browser.contentDocument : null;
  let imgs = doc && !doc.URL.startsWith("http") ? doc.images : [];
  for (let img of imgs)

I determined the bug is that getBrowser() returns the wrong browser here. It is the one of the prefs tab (the .URL is "about:preferences", not the browser of the message preview (a mailbox:// URL).

Squib, would you have an idea how to get the browser of the gMessageDisplay.displayedMessage, not of the currenly focused tab?
Status: NEW → ASSIGNED
Flags: needinfo?(squibblyflabbetydoo)
Version: unspecified → Trunk

Comment 2

2 years ago
Why not just `document.getElementById('messagepane')`?
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 3

2 years ago
What if there are multiple tabs with the 3pane open? Or a separate message in a tab?

Comment 4

2 years ago
There's exactly one three-pane per window. All the tabs with messages in them (three-panes, two-panes, and message tabs) are phony. When you switch tabs, we just show/hide the appropriate panes and load the relevant data into it. It's why scroll position inside a message isn't retained when you switch tabs.
(Assignee)

Comment 5

2 years ago
OK, thanks.
(Assignee)

Comment 6

2 years ago
Created attachment 8610759 [details] [diff] [review]
patch

Actually getBrowser() already contained finding messagepane by ID :)
Attachment #8610759 - Flags: review?(squibblyflabbetydoo)

Comment 7

2 years ago
Comment on attachment 8610759 [details] [diff] [review]
patch

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

I haven't tested this, so I'm assuming it works. See comments below.

::: mail/base/content/mailWindow.js
@@ +455,5 @@
> + * @param aMessagePane  Boolean indicating whether browser for the message pane
> + *                      is requested. In that case the window global message
> + *                      pane browser is returned.
> + */
> +function getBrowser(aMessagePane = false)

Instead of making a function that does two different behaviors, I think it would make more sense to have two functions. "getBrowser(true)" isn't exactly easy to understand without looking at getBrowser's definition. Something like this might be better:

function getBrowser() {
  let tabmail = document.getElementById('tabmail');
  return tabmail ? tabmail.getBrowserForSelectedTab() :
                   getMessagePaneBrowser();
}

function getMessagePaneBrowser() {
  return document.getElementById("messagepane");
}
Attachment #8610759 - Flags: review?(squibblyflabbetydoo) → review-
(Assignee)

Comment 8

2 years ago
Created attachment 8614257 [details] [diff] [review]
patch v1.1

Thanks, good idea.
Attachment #8610759 - Attachment is obsolete: true
Attachment #8614257 - Flags: review?(squibblyflabbetydoo)

Updated

2 years ago
Attachment #8614257 - Flags: review?(squibblyflabbetydoo) → review+
(Assignee)

Comment 9

2 years ago
Thanks.
Keywords: checkin-needed

Comment 10

2 years ago
Is Thunderbird 38 affected by this?
(Assignee)

Comment 11

2 years ago
Yes, the buggy code is in TB38, however the problems probably can't be really called regressions. The ".invalid()" typo is from 2011 and it looks like the line couldn't have ever worked (unless they renamed the XUL treebox method). The "imgs" part does not actually happen unless you enable Preferences in tab, which is not default yet.

Comment 12

2 years ago
https://hg.mozilla.org/comm-central/rev/925c9ae32d4e

Comment 13

2 years ago
https://hg.mozilla.org/comm-central/rev/925c9ae32d4e
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
You need to log in before you can comment on or make changes to this bug.