44915 - 3pane Accelerators: File menu should match spec

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P3)

Description

[Ninoschka Baca]

Overview: In the 3pane window the accelerators for the File menu should match 
the spec.

Comment 1

[timeless]

this might already be done

Comment 2

[timeless]

*** Bug 44923 has been marked as a duplicate of this bug. ***

Comment 3

[scottputterman]

It looks like we have Print Preview, Print, New Message, New Blank Page to Edit, 
and Save As File left. (New Message works but there's no accelerator listed).

Comment 4

[lchiang]

per mail triage:  remove nsbeta3 nomination and move to future milestone.  For 
the short term, we will combine items here into three bugs (in 3pane) to address 
for nsbetat3:  1. delete, change text, move menuitems;  2. enable/disable and 
text changes on context (dynamic);  3.  accelerator keys to work (ie. what is 
shown in the menus actually does something)

When this bug gets revisited after the first release, we'll need to see what 
items remaining that need to be addressed.

Comment 5

[lchiang]

my mistake.  Correcting milestone back to previous one set.  Netscape6 doesn't 
need this by nsbeta3 so leaving the nsbeta3 nomination off.  This can be fixed 
at any time designated by timeless for Mozilla.

Comment 6

[timeless]

Attachment: Open attachments patch r=putterman

Comment 7

[timeless]

Brendan, r=putterman
Please a= [I intend to commit this before branch]. 

Comment 8

[Brendan Eich [:brendan]]

+function file_init()
+{
+       file_attachments();
+       return true;
+}
+
+function file_attachments()
+{
+       var apChild=document.getElementById('attachmentPopup').cloneNode(true);
+       if (!apChild)
+               return false;
+       apChild.removeAttribute('popupanchor');
+       apChild.removeAttribute('popupalign');
+       var amParent=document.getElementById('fileAttachmentMenu');
+       if (!amParent)
+               return false;
+       if (apChild.childNodes.length){
+               if ( amParent.childNodes.length )
+                       amParent.removeChild(amParent.childNodes[0]); 
+               amParent.appendChild(apChild);
+               amParent.removeAttribute('hidden');
+       }
+               else
+               amParent.setAttribute('hidden',true);
+       }
+       return true;
+}
+
Looks buggy to me -- there is an unmatched closing brace after the else (besides
some whacky indentation and hard tabs, which should be expanded per the emacs
modeline comment).  Does this really work?

Also, why is the return value not propagated in file_init?

No a= for you! (yet;-)

/be

Comment 9

[timeless]

Attachment: brendan asked me to clean out the tabs, so i did. Please approve.

Comment 10

[Brendan Eich [:brendan]]

Bracing looks better too.  How about a cvs diff -wu attachment for re-review?

/be

Comment 11

[scottputterman]

The version I received before some of the changes for checking to make sure the 
items were really in the document before using them worked fine.

Comment 12

[Brendan Eich [:brendan]]

what about the unused return value in file_init?  If intentional, please comment
why it's ignored.

/be

Comment 13

[timeless]

Attachment: cvs diff -wu attachment for re-review

Comment 14

[timeless]

Ok, the reason (which I did suspect when I wrote my original code, but was too 
lazy to verify until now) is:

<menu value="click me" oncreate="return false;">
 <menupopup>
  <menuitem value="you will not see this"/>
 </menupoup>
</menu>

My code returns an error value which might be useful in the future, but should 
not actually affect the display of the file menu (attaching another patch that 
ignores the return value).

Comment 15

[timeless]

Attachment: This patch allows the file menu to display even if a strange error case happens

Comment 16

[Brendan Eich [:brendan]]

I gave an a= via IRC.

/be

Comment 17

[timeless]

this fix was checked in a long time ago. verifyme

Comment 18

[lchiang]

per irc chat w/timeless, File | Attachments menu is there for Win32 builds.  Pls
verify on other platforms.

Comment 19

[Ninoschka Baca]

Build 2001-01-22-05: NT4, Mac 9.04
Build 2001-01-19-08: Linux 6.2
Verified Fixed, for accelerators (aka shortcut keys, ie Ctrl+P)