3pane Accelerators: File menu should match spec

VERIFIED FIXED in M18

Status

P3
normal
VERIFIED FIXED
19 years ago
10 years ago

People

(Reporter: nbaca, Assigned: timeless)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

19 years ago
Overview: In the 3pane window the accelerators for the File menu should match 
the spec.
(Reporter)

Updated

19 years ago
Keywords: nsbeta3, ui
QA Contact: lchiang → nbaca

Updated

19 years ago
Target Milestone: --- → M18
(Assignee)

Comment 1

18 years ago
this might already be done
Assignee: putterman → timeless
OS: Windows NT → All
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

18 years ago
*** Bug 44923 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

18 years ago
Depends on: 15096

Comment 3

18 years ago
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

18 years ago
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

Comment 5

18 years ago
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
(Assignee)

Comment 6

18 years ago
Created attachment 14740 [details] [diff] [review]
Open attachments patch r=putterman
(Assignee)

Comment 7

18 years ago
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
(Assignee)

Comment 9

18 years ago
Created attachment 14754 [details] [diff] [review]
brendan asked me to clean out the tabs, so i did. Please approve.
Bracing looks better too.  How about a cvs diff -wu attachment for re-review?

/be

Comment 11

18 years ago
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
(Assignee)

Comment 13

18 years ago
Created attachment 14762 [details] [diff] [review]
cvs diff -wu attachment for re-review
(Assignee)

Comment 14

18 years ago
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).
(Assignee)

Comment 15

18 years ago
Created attachment 14763 [details] [diff] [review]
This patch allows the file menu to display even if a strange error case happens
I gave an a= via IRC.

/be
(Assignee)

Updated

18 years ago
Keywords: helpwanted, mozilla0.9
Target Milestone: M18 → mozilla0.9
(Assignee)

Comment 17

18 years ago
this fix was checked in a long time ago. verifyme
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Keywords: approval, helpwanted, mozilla0.9, patch, ui → verifyme
Resolution: --- → FIXED
Target Milestone: mozilla0.9 → M18

Comment 18

18 years ago
per irc chat w/timeless, File | Attachments menu is there for Win32 builds.  Pls
verify on other platforms.
(Reporter)

Comment 19

18 years ago
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

Updated

10 years ago
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.