1135720 - open email in a new window - Go > Folder functionality

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Anje]

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150122214805

Steps to reproduce:

right click on email
select; Open message in new window

Email opens in new window.
From the 'Menu Bar':
Go > Folder >  select any mail account folder.


Actual results:

Nothing happens. Same opened email still displays.


Expected results:

As this 'GO > Folder ' is not greyed out as a function.
I expected the opened window to open in the selected folder and focus is placed on opening the newest email in the list of the selected folder, so enabling me to use eg: 'Go' > 'Next' to see another email in list of the selected folder.

Note; 
this was also tested using the 'Menu Icon > Go > Folder' route with same results. 
It was also tested in Thundebird Safe Mode with same result.

If this 'Go > Folder' functionality should work in this particular 'Open message in new window' option, then it does not and needs a fix.
If this functionality was never designed to work when email is opened using 'Open message in new window', then it should be greyed out in the 'Menu Bar' and the 'Menu Icon' to stop confusion.

Comment 1

[Wayne Mery (:wsmwk)]

See this in error console?

Error: TypeError: gFolderTreeView is null
Source File: chrome://messenger/content/messageWindow.xul
Line: 1

Comment 2

[:aceman]

Yes, I do see it.
I would just disable that item (and similar ones, that also break, like New->Subfolder).

Comment 3

[WADA:World Anti-bad-Duping Agency]

"goFolderMenu" under "menu_Go" is defined at here, and is shared by messenger window and MailWindow8staand alone message window).
> http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.xul#2622
> <menu xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>    id="goFolderMenu" label="Folder" accesskey="O"
>    oncommand="gFolderTreeView.selectFolder(event.target._folder, true)">
>    <menupopup id="menu_GoFolderPopup" type="folder"
>       showFileHereLabel="true" showRecent="true" recentLabel="Recent" recentAccessKey="R"/></menu>

(Case-A) 
   Open a mail in FolderX in new Window,  Go/Folder, at folder picker, choose different folder from FolderX, Go/Next => next mail in FolderX is shown
(Case-B) 
   Open a mail in FolderX in new Tab,  Go/Folder, at folder picker, choose different folder from FolderX => Tab is closed, Folder Pane is unchanged.
   Follwing is shown at Error Console.(Tb 31.3.0) 
      Error: TypeError: this.search is null
      Source File: resource://gre/modules/dbViewWrapper.js Line: 718
(Case-C) 
   Open a mail in FolderX in new Tab,  switch to Trouble Shooting Information Tab, 
   Go/Folder, at folder picker, choose different folder from FolderX, call FolderY => Nothing occurs
   Swich to 3 pane Tab  => Folder Y is selected at folder pane

For Case-A: 
As known by oncommand="gFolderTreeView.selectFolder(event.target._folder, true)", this is for change of Folder Selection at Folder Pane of 3 pane window.
In MailWindow, Go/Folder manu should be disabled.
Even if selected folder at MailWindow is changed by Go/Folder, it merely produces confusions in next operation such as "Go/Next menu".

For Case-B: I can say nothing.
(i)  Because "mail display at Tab" is "display of a mail in currently selected folder at Folder Pane", if selected folder is changed, Tab should be closed.
(ii) Even when selected folder is changed at folder pne, mail display at Tab shouldn't be affexted.

Comment 4

[:aceman]

Attachment: patch

So here I hide some folder items from the menu as they do not work and I am not sure what they should do even if we wanted to make them work. When user has a message opened it is not sure if he is still aware in which folder he is (there may be multiple folder panes open in multiple windows). Yes, some of the resulting dialogs show which folder/account is used. But it seems to me it could be confusing at times. Why show context items for folders when we aren't obviously in a folder or a folder tree?

I also disable some chat items if there is no 'chatHandler' available. I don't know if it can become available so for safety I do not hide them completely so they get automatically enabled if chatHandler becomes defined.

I convert some of the menu items to use proper commands and get them enabled/disabled/executed in mail3PaneWindowCommands.js (expect those that need an 'event'). Even though I hide some of the items completely, the disabling should not hurt in case there is some other window/overlay that I missed and again exposes those commands when not really ready.

Also, I get errors like "Error: uncaught exception: No connected account!"
when I try "File->New->Chat contact" in the main 3-pane window, when I am in no chat. So that could be even more improved in another bug (adding some more conditions to when to enable the item).

Comment 5

[WADA:World Anti-bad-Duping Agency]

For following exception. 
     Error: TypeError: this.search is null
      Source File: resource://gre/modules/dbViewWrapper.js Line: 718

    // kill off the view and its search association
    if (this.dbView) {
      this.listener.onDestroyingView(false);
      this.search.dissociateView(this.dbView); <= exception at here
      this.dbView.setTree(null);
      this.dbView.selection = null;
      this.dbView.close();

When Obj.PopA.ProbB is accessed, if existence of Obj.PropA is not guranteed alway, code should be "if(Obj && Obj.PropA ) Obj.PropA.PropB=xx;". like one.

Comment 6

[:aceman]

How do you get that error?

Comment 7

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #6)
> How do you get that error?

Question to me?
If so, read comment #3 Case-B, please. Tab close may be a result of the exception, but Tab close may be design.

Comment 8

[:aceman]

OK, I see it. But that needs to repair the functionality of going to a folder, not just hiding unneeded items. I do not even understand why that code you pasted is run (but it is). Please file that as a new bug. Thanks.

Comment 9

[:aceman]

I mean, I tried it, but just adding "if (this.search)this.search.dissociateView(this.dbView); " doesn't help here, it fails on the next line with "this.dbView is null", even though we already tested it to NOT be null a line above. That is weird and needs more investigation.

Comment 10

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #9)
> but just adding "if  (this.search)this.search.dissociateView(this.dbView); " doesn't help here,
> it fails on the next line with "this.dbView is null", even though we already tested it to NOT be null a line above.

That code is "this.dbView.setTree(null)". Even if gDBView object is defined, properties may not be initialized, and .setTree(null) is function call. If setTree is undefined, "setTree is not function" error occurs. So, code like "if gDBView && gDBView.setTree  && (typeof gDBView.setTree)=="function" ) ..."  is needed for tolerance with exceptional situation.
I think gDBView is not defined iif Trouble Shooting Tab(about:support). gDBView is perhaps defined but not initialized as expected if Message display in Tab of messenger. 
gDBView is for Tab of 3 pane window of messenger. How about disabling Go/Folder menu at other Tab than 3 pane display?. I don't think Go/Folder at Trouble Shooting Information Tab, Global Search result Tab etc., is needed. Go/Folder menu should be enabled only when Folder pane/Thread Pane is shown.

Comment 11

[:aceman]

What if the window is the 3-pane window but there is another tab with a single message shown (your case-B)? Currently, using go->Folder closes that tab as it if wanted to go back to the 3-pane tab and show the folder, but it fails there. Do you think we should disable go-Folder if we are in that message pane?

Comment 12

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #11)

If other than messenger window, Go/Folder should be disabled, because there is not Folder Paane/Thread Pne.
If messenger window, Go/Folder menu should be disabled unless current Tab is 3 pane window which has Folder Pane/Thread Pane.
    Multiple Tabs for 3 pane  window can be opened in a messenger window. Other Tabs can be opened in the messenger window.
    Tab #1 : account1/FolderA is selected at folder pane, and mails in account1/FolderA is shown at Thread Pane
                   If Go/Folder is done when this Tab #1 is actve and accountX/FolderX is selected at folder picker,
                   accountX/FolderX is selected at Folder pane of this Tab #1, and content of accountX/FolderX is sihown at Thread pane of this Tab #1.
    Tab #2 : account2/FolderB is selected at folder pane, and mails in account2/FolderB is shown at Thread Pane.
                   If Go/Folder is done when this Tab #2 is actve and accountY/FolderY is selected at folder picker,
                   accountY/FolderY is selected at Folder pane of this Tab #2, and content of accountX/FolderY is shown at Thread pane of this Tab #2.
    Tab #3 : account3/FolderC is selected at folder pane, and mails in account3/FolderC is shown at Thread Pane.
                   If Go/Folder is done when this Tab #4 is actve and accountZ/FolderZ is selected at folder picker,
                   accountZ/FolderZ is selected at Folder pane of this Tab #3, and content of accountZ/FolderZ is shown at Thread pane of this Tab #3.
This is Go/Folder. This is same as "folder click at Folder pane at a Tab where 3 pane window is shown in a messenger window" == Folder Open.
"Go/Folder" is to change selected folder at folder pane of this Tab, and to change View at Thraed pane according to Folder selection change at folder pane.
So, if other than 3 pane window is shown at active Tab, for exmple Trouble Shooting Information, a maail, Global Search result, there is no need to do "selected folder switch at Folder pane" nor "View chane of Thread pane".

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 8568797 [details] [diff] [review]
patch

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

::: mail/base/content/messageWindow.js
@@ +531,5 @@
>  
>  function HideMenus()
>  {
> +  // TODO: Seems to be a lot of repetitive code.
> +  // Can we just fold this into an array of element IDs and loop over them?

I wouldn't bother

Comment 14

[:aceman]

Thanks.

Comment 15

[Kent James (:rkent)]

Comment on attachment 8568797 [details] [diff] [review]
patch

http://hg.mozilla.org/comm-central/rev/e403c3a6938f

Push to aurora after a nightly cycle.

Comment 16

[Kent James (:rkent)]

Comment on attachment 8568797 [details] [diff] [review]
patch

http://hg.mozilla.org/releases/comm-aurora/rev/d6ae5fb08f51