887183 - "GetPromptService is not defined" errors in Error Console

Status: RESOLVED FIXED

(SeaMonkey :: Composer, defect)

Description

[Phoenix]

While experimenting with reproducing Bug 886967, in some cases I get "GetPromptService is not defined" errors in Error Console (no solid steps to reproduce), for example
Error: An error occurred executing the cmd_close command: [Exception... "'[JavaScript Error: "GetPromptService is not defined" {file: "chrome://editor/content/editor.js" line: 647}]' when calling method: [nsIControllerCommand::doCommand]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://global/content/globalOverlay.js :: goDoCommand :: line 91"  data: yes]
Source File: chrome://global/content/globalOverlay.js
Line: 95
In editor.js on line 647
var promptService = GetPromptService();
but it looks like that this function was removed in Bug 732807 (and some changes in near code was in Bug 718480) so this part should be reworked.

Comment 1

[Philip Chee]

>  var promptService = GetPromptService();
Change this to:
  var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].getService(Components.interfaces.nsIPromptService);
OR
  var promptService = Services.prompt;
(I see Services.jsm is already available)

Comment 2

[Phoenix]

Attachment: One line fix

Okay, here it goes

Comment 3

[Phoenix]

Comment on attachment 767718 [details] [diff] [review]
One line fix

Changed reviewer per Ratty's comment in irc

Comment 4

[[e-mail]]

(In reply to Phoenix from comment #0)
> In editor.js on line 647
> var promptService = GetPromptService();
> but it looks like that this function was removed in Bug 732807 (and some
> changes in near code was in Bug 718480) so this part should be reworked.

Bug 795158 actually.

(In reply to Philip Chee from comment #1)
> >  var promptService = GetPromptService();
> Change this to:
>   var promptService =
> Components.classes["@mozilla.org/embedcomp/prompt-service;1"].
> getService(Components.interfaces.nsIPromptService);
> OR
>   var promptService = Services.prompt;
> (I see Services.jsm is already available)

Bug 795158 style is to replace the uses of promptService with Services.prompt e.g. Services.prompt.BUTTON_TITLE_OK

Comment 5

[Philip Chee]

> Bug 795158 style is to replace the uses of promptService with Services.prompt e.g.
> Services.prompt.BUTTON_TITLE_OK
Yes but in this case I feel this is unnecessarily verbose.

Comment 6

[Ian Neal]

Comment on attachment 767718 [details] [diff] [review]
One line fix

My preference is to change all instances of promptService to Services.prompt
r=me with that done.

Comment 7

[Phoenix]

Attachment: Aligned with Bug 795158 style [check-in comment 13]

Done, also replaced some vars with lets as in Bug 795158, please check

Comment 8

[Philip Chee]

> +  let result = {value:0};
> +  let promptFlags = Services.prompt.BUTTON_TITLE_CANCEL * Services.prompt.BUTTON_POS_1;
> +  let button1Title = null;
> +  let button3Title = null;
Almost right. The style in this file uses "var" rather than "let".

Comment 9

[Phoenix]

On other hand, around Services.prefs in this files there are isles of lets :)

Comment 10

[Philip Chee]

> On other hand, around Services.prefs in this files there are isles of lets :)
oh well, if that's the case...

Comment 11

[Ian Neal]

Comment on attachment 768880 [details] [diff] [review]
Aligned with Bug 795158 style [check-in comment 13]

As this is shared code, you will probably need a review from the likes of standard8 too.

Comment 12

[[e-mail]]

Actually it's not really shared code.

There's one caller in editingOverlay.js which isn't shared code.

There are four callers in ComposerCommands.js but three are validate, send page and preview, which make no sense in message compose, and one is close, which has separate code in message compose.

Comment 13

[Philip Chee]

Comment on attachment 768880 [details] [diff] [review]
Aligned with Bug 795158 style [check-in comment 13]

Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/8fac43cf94da

[Approval Request Comment]
Regression caused by (bug #): Bug 795158 
User impact if declined: Prompt to publish webpage is broken
Testing completed (on m-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): Low bustage fix.
String changes made by this patch: None.

Comment 14

[[e-mail]]

Comment on attachment 768880 [details] [diff] [review]
Aligned with Bug 795158 style [check-in comment 13]

Actually it affects cmd_close too, so that's potential dataloss (window silently closes without prompting to save/publish).

Comment 15

[Philip Chee]

Pushed to branches:
https://hg.mozilla.org/releases/comm-aurora/rev/c4b5d8f493b5
https://hg.mozilla.org/releases/comm-beta/rev/993c83510065