Support some actions of WM_APPCOMMAND such as New, Open, Close, Save, Find, Help, SendMail, ReplyToMail and ForwardMail ("Close" Multimedia Key on Windows closes browser, not current tab)

NEW
Assigned to

Status

SeaMonkey
OS Integration
5 years ago
2 years ago

People

(Reporter: Philip Chee, Assigned: waffles, Mentored)

Tracking

Trunk
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
From Bug 672193 Comment 8:

> Next, we should need to add new command atoms for New, Open, Close, Find, Help, 
> Print, Send Mail, Forward Mail and Reply To Mail.
> 
> Firefox should do nothing for Forward Mail and Reply To Mail. So, nsWindow 
> should check the result of the command event. And command event handler should 
> consume the events if it handles the events.
> 
> New and Close cause opening or closing tab on other browsers (IE10, Opera and 
> Chrome). So, we should handle the manipulation target is current tab.
(Reporter)

Comment 1

5 years ago
See https://hg.mozilla.org/mozilla-central/rev/bac88e283278 for details.
(Reporter)

Comment 2

5 years ago
> See https://hg.mozilla.org/mozilla-central/rev/bac88e283278 for details.

1. We have BrowserOpenTab()
2. We have BrowserCloseTabOrWindow()
3. for gFindBar.onFindCommand()
   We can use BrowserFind() which calls findInPage(getFindInstData()) which calls findbar.onFindCommand().
4. openHelpLink('firefox-help')
   We can open our in-application help via toolkit function openHelp();
   See http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/help/content/contextHelp.js#12
5. We have BrowserOpenFileWindow()
6. We have PrintUtils.print()
7. saveDocument(window.content.document) This is a toolkit function so we have that.
8.
> +  case "SendMail":
> +    MailIntegration.sendLinkForWindow(window.content);
For this we can use function sendPage(aDocument) (from mailNavigatorOverlay.js)

We might also want to extend HandleAppCommandEvent() in:
1. msgMail3PaneWindow.js
2. messageWindow.js.
(Reporter)

Updated

5 years ago
Whiteboard: [good first bug][lang=js][mentor=Philip.Chee(Ratty)]
(Reporter)

Updated

5 years ago
Component: General → OS Integration
(Reporter)

Updated

5 years ago
Duplicate of this bug: 830234

Updated

4 years ago
Mentor: philip.chee@gmail.com
Whiteboard: [good first bug][lang=js][mentor=Philip.Chee(Ratty)] → [good first bug][lang=js]

Comment 4

3 years ago
hi! I'm new here , Please assign this bug to me!

Comment 5

3 years ago
Hi, Abhirav, and welcome aboard. We'll typically assign a bug to a new contributor as soon as they attach a preliminary patch to the bug. If you have any questions getting set, please join us on IRC in the #introduction channel, and good luck!

Comment 6

3 years ago
sir, what is your nick on the IRC , I couldn't find u in the user list . Please help me get started as this is my 1st attempt, thank you!
(Reporter)

Comment 7

3 years ago
Hi!

In addition to #introduction you should also use #SeaMonkey for SeaMonkey specific developer questions.

We'll assign the bug to you when you've started work on a patch.
Flags: needinfo?(abhirav.kariya)

Comment 8

3 years ago
Hi!

I am new here... Where can I find the full code... Is there any repository???
(Reporter)

Comment 9

3 years ago
(In reply to Kousik Satish from comment #8)
> Hi!
> 
> I am new here... Where can I find the full code... Is there any repository???

Please read this first:
https://developer.mozilla.org/en-US/docs/Simple_SeaMonkey_build

SeaMonkey code resides in http://hg.mozilla.org/comm-central/ and also makes use of code from https://hg.mozilla.org/mozilla-central/

Comment 10

3 years ago
(In reply to Philip Chee from comment #9)
 
> Please read this first:
> https://developer.mozilla.org/en-US/docs/Simple_SeaMonkey_build
> 
> SeaMonkey code resides in http://hg.mozilla.org/comm-central/ and also makes
> use of code from https://hg.mozilla.org/mozilla-central/

Thank you very much...! I will check it out...!
(Assignee)

Comment 11

3 years ago
Hey Philip, I think I can work on this bug. I've already worked on a few bugs and so we can directly jump into the bug itself. So, what are we doing here? Can you get me started?
Flags: needinfo?(philip.chee)
(Reporter)

Comment 12

3 years ago
First read comment 0 comment 1 and comment 2 and then for some background Firefox/Core bug 825596.

This is the Firefox version of HandleAppCommandEvent()
http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/browser.js?rev=197f69574d75#1764
And here is SeaMonkey's:
http://mxr.mozilla.org/comm-central/source/suite/browser/navigator.js?rev=83106028dd34#401

What we want is to extend the SeaMonkey version to match Firefox. The names of the Firefox functions are not always the same as SeaMonkey so you might need to do some adjustments.
Assignee: nobody → wafflespeanut
Flags: needinfo?(philip.chee)
Flags: needinfo?(abhirav.kariya)
(Assignee)

Comment 13

3 years ago
Created attachment 8612486 [details] [diff] [review]
appevents.patch

It's just my initial patch (and I haven't done much - only the switch cases). I have a few questions. Are there any arguments to the `openHelp()` function call? Because, Firefox has passed a topic, and I don't know the topic name for Sea Monkey. And, that `aDocument` we pass into `sendPage()`, does it work for `msgMail3PaneWindow.js` and `messageWindow.js`? Apart from these, should I add any comments regarding those functions?
Attachment #8612486 - Flags: review?(philip.chee)
(Assignee)

Comment 14

3 years ago
Oh, and I forgot something. The changeset you showed in comment #1 has revisions made to "/mozilla/widget/windows/nsWindow.cpp" and "/mozilla/dom/base/nsGkAtomList.h". While the header file already has the necessary changes, the CPP file doesn't. Am I supposed to revise it?
Flags: needinfo?(philip.chee)
(Reporter)

Comment 15

3 years ago
Hi! Sorry for the slow response. Anything under /mozilla/ is code in mozilla-central which we should not touch in *this* bug. This does mean that we are limited at the moment to the "atoms" in those files. But the current list should be sufficient for this bug at least.
Flags: needinfo?(philip.chee)
(Assignee)

Comment 16

3 years ago
No problem Philip. I do understand the reason for your slow response. But, you haven't reviewed my patch yet :)
Flags: needinfo?(philip.chee)
(Reporter)

Comment 17

2 years ago
Comment on attachment 8612486 [details] [diff] [review]
appevents.patch

> +++ b/suite/browser/navigator.js

> +    case "Help":
> +      openHelp();

function openHelp(topic, contentPack)
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/help/content/help.js?rev=58fe9dc85fa4#65
The default topic is "welcome". So we just need to specify the contentPack. In our case this is:
"chrome://communicator/locale/help/suitehelp.rdf"
openHelp("", "chrome://communicator/locale/help/suitehelp.rdf")

> +    case "Print":
> +      PrintUtils.print();
Not your fault but we are switching to the new Print API
See Bug 1146454
https://hg.mozilla.org/mozilla-central/rev/dfd2a128d19c
and Bug 1195863
https://bug1195863.bmoattachments.org/attachment.cgi?id=8649372
You'll need something like this:
PrintUtils.printWindow(gBrowser.selectedBrowser.outerWindowID,
                       gBrowser.selectedBrowser);

> +    case "Save":
> +      saveDocument(window.content.document);
This will work but we can use the new API introduced in Bug 1141337
         saveBrowser(gBrowser.selectedBrowser)

> +++ b/suite/mailnews/messageWindow.js

I think some of these aren't appropriate.

>      case "Home": 
> +      goDoCommand('cmd_goStartPage');
Please remove this.

> +    case "New":
> +      BrowserOpenTab();
Please remove this.

> +    case "Close":
> +      BrowserCloseTabOrWindow();
Looking at File->Close we should be using
goDoCommand("cmd_close");

> +    case "Find":
> +      BrowserFind();
Use goDoCommand("cmd_find");

> +    case "Help":
> +      openHelp();
openHelp("mail", "chrome://communicator/locale/help/suitehelp.rdf");

> +      break;
> +    case "Open":
> +      BrowserOpenFileWindow();
(1) This is wrong.
(2) Also BrowserOpenFileWindow() isn't available in this window.
(3) File->Open calls MsgOpenFromFile() so use this instead.

> +    case "Print":
> +      PrintUtils.print();
Please remove this.

> +    case "Save":
> +      saveDocument(window.content.document);
> +      break;
Please remove this.

> +    case "SendMail":
> +      sendPage(aDocument);
Please remove this.


> +++ b/suite/mailnews/msgMail3PaneWindow.js

I think some of these aren't appropriate.

> +    case "New":
> +      BrowserOpenTab();
File->New in this window has several more or less equal options.
Please remove this.

> +    case "Close":
> +      BrowserCloseTabOrWindow();
Looking at File->Close we should be using
goDoCommand("cmd_close");

> +    case "Find":
> +      BrowserFind();
Please remove this. Find in message depends on whether a message is open and focused or not.

> +    case "Help":
> +      openHelp();
openHelp("mail", "chrome://communicator/locale/help/suitehelp.rdf");

> +    case "Open":
> +      BrowserOpenFileWindow();
(1) This is wrong.
(2) Also BrowserOpenFileWindow() isn't available in this window.
(3) File->Open calls MsgOpenFromFile() so use this instead.

> +    case "Print":
> +      PrintUtils.print();
(1) There should be a break;
(2) Please remove this item.
(3) Printing a message depends on whether a message is open and focused or not.

> +    case "Save":
> +      saveDocument(window.content.document);
Please remove this.

> +    case "SendMail":
> +      sendPage(aDocument);
Please remove this.
Flags: needinfo?(philip.chee)
Attachment #8612486 - Flags: review?(philip.chee) → review-
(Assignee)

Comment 18

2 years ago
Meh, I can't even recall whether it's me who really did this! (quite embarrassing indeed) :P
Anyways, I'll finish it off by this weekend :)
You need to log in before you can comment on or make changes to this bug.