I was asked to port Bug 581470 from Thunderbird to add Ctrl+P and Ctrl+W shortcuts to the Print Preview window as implemented in TB.
Remind me to actually TEST patches before I propose them!
Note that the traditional way of doing this in SeaMonkey is to overlay utilityOverlay.xul which provides you with the definitions of key_close and key_print. (key_close lives in platformCommunicatorOverlay.xul but that's just an implementation detail.) Also note that keys don't work unless you put them in a keyset.
Hmm.. This implementation does work but if I am in essence replicating already defined methods I should do it that way. I will look into it. I just hope I can get a handle of implementing things like this the proper way.
Seems utilityOverlay is already present. Simply adding key_close works fine with or without an enclosing keyset tag. However, key_print doesn't function at all from the print preview window. Though alt+P does start printing so do we actually want ctrl+P? At any rate I do think key_close needs to be on every window for consistency.
Comment on attachment 8422075 [details] [diff] [review] patch.diff So the key bit is this: <!-- Provide shortcut keys for toolkit's print preview; commands will be overridden by printUtils.js --> Looking at the source for onKeyPressPP: http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/printing/content/printUtils.js?rev=5959f80e1831#292 We see that the keypress listener: (1) Grabs the event before it can reach our key handlers in utilityOverlay and does it's own processing for these two keys. (2) is hard coded to look for "printKb" Which is why <key id="key_print"> doesn't work. (3) Note onKeyPressPP looks for both ALT-W and CTRL-W, ALT-P and CTRL-P Suggestion 1: minimal changes needed: ------------------------------------- utilityOverlay.xul: <key id="key_print" key="&printCmd.key;" command="cmd_print" modifiers="accel"/> + <key id="printKb" + key="&printCmd.key;"/> msgPrintEngine.xul: + <!-- Provide shortcut keys for toolkit's print preview; commands will be overridden by printUtils.js --> + <key id="printKb"/> + <key id="key_close"/> Plus remove printEngine.dtd since we don't need it any more. Suggestion 2: ------------------------------------- Fix the toolkit printUtils.js and cut down on the insanity.
Attachment #8422075 - Flags: feedback?(philip.chee) → feedback-
Comment on attachment 8422075 [details] [diff] [review] patch.diff Nothing to add on top of what Philip said. Short term fix is suggestion 1, long term fix is suggestion 2. I would do suggestion 1 within this bug and spin off the long term fix to another bug.
Attachment #8422075 - Flags: review?(iann_bugzilla) → review-
So the consensus is Suggestion 1: > utilityOverlay.xul: > <key id="key_print" > key="&printCmd.key;" > command="cmd_print" > modifiers="accel"/> > + <key id="printKb" > + key="&printCmd.key;"/> Maybe add |modifiers="accel"| > msgPrintEngine.xul: > + <!-- Provide shortcut keys for toolkit's print preview; commands will be overridden by printUtils.js --> > + <key id="printKb"/> > + <key id="key_close"/> > Plus we don't need to add printEngine.dtd at all. Tobin, care to whip up a new patch?
Understood.. Stay tuned for that while I pull latest c-c and get that patch created.
Tobin has moved on to Palemoon. So I'm taking over this bug. This is the minimal patch discussed in previous comments.
Pushed to comm-central http://hg.mozilla.org/comm-central/rev/3a8e98f52f91
You need to log in before you can comment on or make changes to this bug.