Closed Bug 44915 Opened 20 years ago Closed 19 years ago

3pane Accelerators: File menu should match spec

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: nbaca, Assigned: timeless)

References

()

Details

Attachments

(4 files)

Overview: In the 3pane window the accelerators for the File menu should match 
the spec.
Keywords: nsbeta3, ui
QA Contact: lchiang → nbaca
Target Milestone: --- → M18
this might already be done
Assignee: putterman → timeless
OS: Windows NT → All
Status: NEW → ASSIGNED
*** Bug 44923 has been marked as a duplicate of this bug. ***
Depends on: 15096
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).
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.
Keywords: nsbeta3
Target Milestone: M18 → Future
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.
Target Milestone: Future → M18
Brendan, r=putterman
Please a= [I intend to commit this before branch]. 
Keywords: approval, patch
+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
Bracing looks better too.  How about a cvs diff -wu attachment for re-review?

/be
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.
what about the unused return value in file_init?  If intentional, please comment
why it's ignored.

/be
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).
I gave an a= via IRC.

/be
Target Milestone: M18 → mozilla0.9
this fix was checked in a long time ago. verifyme
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: mozilla0.9 → M18
per irc chat w/timeless, File | Attachments menu is there for Win32 builds.  Pls
verify on other platforms.
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)
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.