1658227 - Mark Message or thread as read (shortcuts: M/R) fails from facet search results "show results as list" or Open message in conversation (Uncaught TypeError: can't access property "getNumUnread", this.displayedFolder is null)

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[Wayne Mery (:wsmwk)]

80.0b2. I'm assuming this is a regression but I have no idea when it started.

Steps:

• global search, then show results as list
• OR pick a message and do open message in conversation
• type "R" to mark thread as read
  Results:
  fails. error console below

Works for the same message if I do "open message in containing folder".

20:06:17.736 Uncaught TypeError: this.displayedFolder is null
get canMarkThreadAsRead chrome://messenger/content/folderDisplay.js:2315
isCommandEnabled chrome://messenger/content/mail3PaneWindowCommands.js:484
isCommandEnabled chrome://messenger/content/mailTabs.js:834
isCommandEnabled chrome://messenger/content/tabmail.js:548
goDoCommand chrome://global/content/globalOverlay.js:100
oncommand chrome://messenger/content/messenger.xhtml:1
folderDisplay.js:2315:21
20:06:17.737
An error occurred executing the cmd_markThreadAsRead command: [Exception... "[JavaScript Error: "this.displayedFolder is null" {file: "chrome://messenger/content/folderDisplay.js" line: 2315}]'[JavaScript Error: "this.displayedFolder is null" {file: "chrome://messenger/content/folderDisplay.js" line: 2315}]' when calling method: [nsIController::isCommandEnabled]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: chrome://global/content/globalOverlay.js :: goDoCommand :: line 100" data: yes]

Comment 1

[Wayne Mery (:wsmwk)]

Walt, can you reproduce this?

Comment 2

[Alice0775 White]

STR

1. Ctrl+K (focus search field) and type some search text and hit enter
2. Click on "Show results as list"
3. Focus the list pane and Ctrl+A (select all)
4. Hit r key

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=e258a4e7d2369fef4d2e6850422cd646faedae47&tochange=f8d95023913a43a93dd4a5f9f81fc6732aca8da8
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3d1e9c77a42dec977bd7a22e2668af56b2587145&tochange=7f7b983390650cbc7d736e92fd3e1f629a30ac02

Comment 3

[Alice0775 White]

Error console:

Uncaught TypeError: can't access property "getNumUnread", this.displayedFolder is null
    get canMarkThreadAsRead chrome://messenger/content/folderDisplay.js:2315
    isCommandEnabled chrome://messenger/content/mail3PaneWindowCommands.js:484
    isCommandEnabled chrome://messenger/content/mailTabs.js:834
    isCommandEnabled chrome://messenger/content/tabmail.js:548
    goDoCommand chrome://global/content/globalOverlay.js:100
    oncommand chrome://messenger/content/messenger.xhtml:1
folderDisplay.js:2315:21
    get canMarkThreadAsRead chrome://messenger/content/folderDisplay.js:2315
    isCommandEnabled chrome://messenger/content/mail3PaneWindowCommands.js:484
    isCommandEnabled chrome://messenger/content/mailTabs.js:834
    isCommandEnabled chrome://messenger/content/tabmail.js:548
    goDoCommand chrome://global/content/globalOverlay.js:100
    oncommand chrome://messenger/content/messenger.xhtml:1

An error occurred executing the cmd_markThreadAsRead command: [Exception... "[JavaScript Error: "can't access property "getNumUnread", this.displayedFolder is null" {file: "chrome://messenger/content/folderDisplay.js" line: 2315}]'[JavaScript Error: "can't access property "getNumUnread", this.displayedFolder is null" {file: "chrome://messenger/content/folderDisplay.js" line: 2315}]' when calling method: [nsIController::isCommandEnabled]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://global/content/globalOverlay.js :: goDoCommand :: line 100"  data: yes] globalOverlay.js:104
    goDoCommand chrome://global/content/globalOverlay.js:104
    oncommand chrome://messenger/content/messenger.xhtml:1

Comment 4

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #0)

• type "R" to mark a messages as read

R = mark thread read, not message
M = toggle message read/unread

Comment 5

[Wayne Mery (:wsmwk)]

I've further expanded the functions affected to include mark message as read and open message in conversation.

Apparently we don't have an automated test to cover this?

Comment 6

[Wayne Mery (:wsmwk)]

Or we do have tests, but insufficient.

Note also bug 1658227 has whiteboard "Thunderbird-disabled-test"

Comment 7

[Lasana Murray]

Can we rename this bug to the error encountered? The current title is a bit confusing. Also I think I'm running into a different error (Daily):

    AutocompleteInput chrome://global/content/elements/autocomplete-input.js:70
    openTab chrome://messenger/content/glodaFacetTab.js:36
    openTab chrome://messenger/content/tabmail.js:955
    doSearch chrome://messenger/content/gloda-autocomplete-input.js:217
    MozGlodaAutocompleteInput chrome://messenger/content/gloda-autocomplete-input.js:60````

Comment 8

[Thomas D. (:thomas8)]

(In reply to Lasana Murray from comment #7)

Can we rename this bug to the error encountered? The current title is a bit confusing.

Lasana! Sorry hey, we love verbose bug titles in bug triage because of a systemic shortcoming in Bugzilla where the summary is the only place to add search words which make the bug retrievable and avoid more duplicates. Instead, they should have separate fields for alternative titles and/or search words. I've added the error to the bug title for you ;-)

Also I think I'm running into a different error (Daily):
Uncaught TypeError: can't access property "hasAttribute", window.gBrowser.selectedBrowser is null

Well, that's another one but it happens at step 1 (global search).
Wayne is talking about the error of step 3:
After doing "show results as list", then press "R" on any (unread) msg or thread in the list, and you still get this:

Uncaught TypeError: can't access property "getNumUnread", this.displayedFolder is null
    get canMarkThreadAsRead chrome://messenger/content/folderDisplay.js:2315
    isCommandEnabled chrome://messenger/content/mail3PaneWindowCommands.js:484
    isCommandEnabled chrome://messenger/content/mailTabs.js:834
    isCommandEnabled chrome://messenger/content/tabmail.js:548
    goDoCommand chrome://global/content/globalOverlay.js:100
    oncommand chrome://messenger/content/messenger.xhtml:1
folderDisplay.js:2315:21

An error occurred executing the cmd_markThreadAsRead command: [Exception... "[JavaScript Error: "can't access property "getNumUnread", this.displayedFolder is null" {file: "chrome://messenger/content/folderDisplay.js" line: 2315}]'[JavaScript Error: "can't access property "getNumUnread", this.displayedFolder is null" {file: "chrome://messenger/content/folderDisplay.js" line: 2315}]' when calling method: [nsIController::isCommandEnabled]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: chrome://global/content/globalOverlay.js :: goDoCommand :: line 100" data: yes] globalOverlay.js:104

Comment 9

[Thomas D. (:thomas8)]

Wayne, pressing "M" in your step 3 no longer reproduces for me, does it for you? on daily 82.0a1 (2020-08-25) (64-bit), but you reported against 80.0b2.

• type "R" to mark a messages as read

Can you pls correct comment 0:

• press R to mark thread as read

(and add this you can still reproduce:)

• or press M to mark msg as read

Comment 10

[Wayne Mery (:wsmwk)]

Comment 0 is corrected.

Lassana, do you have a theory for the fix?

Comment 11

[Lasana Murray]

I believe I have fixed it. I'm writing a test to check for it.

Comment 12

[Lasana Murray]

Attachment: bug1658227.patch

If the underlying view has "synthetic" messages, the displayedFolder property will be null causing the error. This patch includes a check for a synthetic view incanMarkThreadAsRead.

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 9174640 [details] [diff] [review]
bug1658227.patch

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

Looks pretty good, but seems like the test fails (due to bug 1663904)

::: mail/base/content/folderDisplay.js
@@ +2314,5 @@
>    get canMarkThreadAsRead() {
> +    let hasUnread =
> +      this.displayedFolder && this.displayedFolder.getNumUnread(false) > 0;
> +
> +    let hasSynthetic =

"has" seems like the wrong verb. it's "is".

But please just do the two checks in the if, otherwise it's a bit confusing. The synthetic view will also be checked for unread, only later on.

::: mail/test/browser/folder-display/browser_syntheticViews.js
@@ +22,5 @@
> +let gAutoRead;
> +
> +function createThreadWithTerm(msgCount, term) {
> +  let thread = create_thread(msgCount);
> +  thread.synMessages.map(msg => {

I think you mean .forEach, not map.

But this would be better

for (let msg of thread.synMessages) {

@@ +105,5 @@
> +
> +add_task(function setUp() {
> +  gAutoRead = Services.prefs.getBoolPref("mailnews.mark_message_read.auto");
> +  Services.prefs.setBoolPref("mailnews.mark_message_read.auto", false);
> +});

Instead of doing this as a task, just do it in global scope. I think otherwise tests could fail when not running sequentially.

Comment 14

[Lasana Murray]

Attachment: bug1658227v2.patch

Updated for feedback. I'll see if I can figure out bug 1663904 there.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9175588 [details] [diff] [review]
bug1658227v2.patch

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

Looks pretty good, but can't test due to the other bug. Reflag for review when the test is working.

Please for all the tests, add jsdoc style comments describing what it will test.

Comment 16

[Lasana Murray]

The tests here pass:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9e026891a6859f97606e2a1411d34366b8b8499c

However they won't work with --verify because the folders created need to be deleted after each test. I looked into doing that but I could not find a friendly way to do that in FolderDisplayHelpers.jsm looks like some refactoring is needed there.

Alternatively, I could give the folders random names though I noticed other tests that create folders don't work with verify either.

Comment 17

[Magnus Melin [:mkmelin]]

You can just call folder.deleteSelf(null); at the end of each test. I don't think anything more elaborate is needed.

Comment 18

[Lasana Murray]

calling folder.deleteSelf(null) in a clean up function results in this:

FAIL uncaught exception - : Component returned failure code: 0x80550006 [nsIMsgFolder.msgDatabase] at _persistColumnStates@chrome://messenger/content/folderDisplay.js:533:39 hintColumnsChanged@chrome://messenger/content/folderDisplay.js:557:10 OnLoadThreadPane/observer<@chrome://messenger/content/msgMail3PaneWindow.js:1332:20 MutationCallback*OnLoadThreadPane@chrome://messenger/content/msgMail3PaneWindow.js:1331:18 InitPanes@chrome://messenger/content/msgMail3PaneWindow.js:1298:3 LoadPostAccountWizard@chrome://messenger/content/msgMail3PaneWindow.js:790:3 _delayedStartup@chrome://messenger/content/msgMail3PaneWindow.js:705:7 EventListener.handleEvent*onLoad@chrome://messenger/content/msgMail3PaneWindow.js:661:12 EventHandlerNonNull*@chrome://messenger/content/messenger.xhtml:286:36

Comment 19

[Lasana Murray]

Attachment: 1658227v3.patch

Updated the tests to run better with --verify. They are all passing using --verify for me right now but I occasionally ran into errors like the one in bug 1662911 and a timeout when clicking tree items from time to time.

Comment 20

[Lasana Murray]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b96dd78a7932a1de3db6f695d7f3176f6ed09bdf

Comment 21

[Magnus Melin [:mkmelin]]

Comment on attachment 9181687 [details] [diff] [review]
1658227v3.patch

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

LGTM, r=mkmelin

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2757dd53eeea
Take synthetic views into consideration when calculating canMarkThreadAsRead. r=mkmelin