DOM Inspector DOM tree viewer context menu lacks several access keys, and has incorrect ones as well (for example, the After menu item under Paste has access key "P"). The attached patch adds and corrects the necessary access keys.

Comment on attachment 283699 [details] [diff] [review] Add the access keys I'm not convinced with your choices of access keys, so here are some suggestions for Shawn to consider :-) > <!ENTITY cmdInspectBrowser.label "Inspect Contained Document"> >+ <!ENTITY cmdInspectBrowser.accesskey "e"> > <!ENTITY cmdInspectInNewWindow.label "Inspect in New Window"> >+ <!ENTITY cmdInspectInNewWindow.accesskey "N"> Maybe these should be m and N, or n and W? > <!ENTITY mnEditPasteReplace.label "In place of (Replace)"> >+ <!ENTITY mnEditPasteReplace.accesskey "I"> > <!ENTITY mnEditPasteAsParent.label "As parent"> >+ <!ENTITY mnEditPasteAsParent.accesskey "r"> R and n, perhaps?

(In reply to comment #1) > > <!ENTITY cmdInspectBrowser.label "Inspect Contained Document"> > >+ <!ENTITY cmdInspectBrowser.accesskey "e"> > > <!ENTITY cmdInspectInNewWindow.label "Inspect in New Window"> > >+ <!ENTITY cmdInspectInNewWindow.accesskey "N"> > Maybe these should be m and N, or n and W? Hmm, justification for e, m, or n for the first one? I don't really see how any of those relate actually. As for the second one, N or W works for me (I'm indifferent here). > > <!ENTITY mnEditPasteReplace.label "In place of (Replace)"> > >+ <!ENTITY mnEditPasteReplace.accesskey "I"> > > > <!ENTITY mnEditPasteAsParent.label "As parent"> > >+ <!ENTITY mnEditPasteAsParent.accesskey "r"> > R and n, perhaps? I like R better, and as for the latter, I can't think of a good one. Justification for r or n?

CCing Neil, and turning his attention to comment 2.

(In reply to comment #2) >(In reply to comment #1) >>> <!ENTITY cmdInspectBrowser.label "Inspect Contained Document"> >>>+ <!ENTITY cmdInspectBrowser.accesskey "e"> >>Maybe these should be m and N, or n and W? >Hmm, justification for e, m, or n for the first one? I don't really see how >any of those relate actually. I = Insert C = Copy D = Delete and we're supposed to avoid vowels. >>> <!ENTITY mnEditPasteAsParent.label "As parent"> >>>+ <!ENTITY mnEditPasteAsParent.accesskey "r"> >>R and n, perhaps? >Justification for r or n? n is wider than r, so the underline is more visible.

Comment on attachment 283699 [details] [diff] [review] Add the access keys Well, I'm fairly indifferent here, so you can use your judgement. r=sdwilsh

Checking in extensions/inspector/resources/content/viewers/dom/dom.xul; /cvsroot/mozilla/extensions/inspector/resources/content/viewers/dom/dom.xul,v <-- dom.xul new revision: 1.19; previous revision: 1.18 done Checking in extensions/inspector/resources/locale/en-US/viewers/dom.dtd; /cvsroot/mozilla/extensions/inspector/resources/locale/en-US/viewers/dom.dtd,v <-- dom.dtd new revision: 1.17; previous revision: 1.16 done

Next time, please unbitrot your over two-month-old patches before requesting approval / checkin-needed. Thanks.

(In reply to comment #7) > Next time, please unbitrot your over two-month-old patches before requesting > approval / checkin-needed. Thanks. You're right, sorry about the problem.

There is a bug here, see this part of patch + <menuitem id="mnEditPasteFirstChild" + label="&mnEditPasteFirstChild.label;" + accesskey="&mnEditPasteFirstChild.accesskey;" + command="cmdEditPasteFirstChild"/> + <menuitem id="mnEditPasteFirstChild" + label="&mnEditPasteFirstChild.label;" + accesskey="&mnEditPasteFirstChild.accesskey;" + command="cmdEditPasteFirstChild"/> Doubled menu-item.

(In reply to comment #9) > There is a bug here, see this part of patch > > + <menuitem id="mnEditPasteFirstChild" > + label="&mnEditPasteFirstChild.label;" > + accesskey="&mnEditPasteFirstChild.accesskey;" > + command="cmdEditPasteFirstChild"/> > + <menuitem id="mnEditPasteFirstChild" > + label="&mnEditPasteFirstChild.label;" > + accesskey="&mnEditPasteFirstChild.accesskey;" > + command="cmdEditPasteFirstChild"/> > > > Doubled menu-item. Oops. You're right. Re-opening... I'm posting a patch right away.

Fix the problem mentioned in comment 9 by removing the mistakenly duplicated menu item...

Checking in extensions/inspector/resources/content/viewers/dom/dom.xul; /cvsroot/mozilla/extensions/inspector/resources/content/viewers/dom/dom.xul,v <-- dom.xul new revision: 1.20; previous revision: 1.19 done