Last Comment Bug 887183 - "GetPromptService is not defined" errors in Error Console
: "GetPromptService is not defined" errors in Error Console
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Composer (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.22
Assigned To: Phoenix
:
Mentors:
: 890911 (view as bug list)
Depends on:
Blocks: 795158
  Show dependency treegraph
 
Reported: 2013-06-26 02:59 PDT by Phoenix
Modified: 2013-07-08 09:36 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
affected
fixed
fixed
fixed


Attachments
One line fix (1.11 KB, patch)
2013-06-26 05:46 PDT, Phoenix
iann_bugzilla: review+
Details | Diff | Review
Aligned with Bug 795158 style [check-in comment 13] (2.44 KB, patch)
2013-06-28 04:49 PDT, Phoenix
iann_bugzilla: review+
neil: approval‑comm‑aurora+
neil: approval‑comm‑beta+
Details | Diff | Review

Description Phoenix 2013-06-26 02:59:51 PDT
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 2013-06-26 05:23:39 PDT
>  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 2013-06-26 05:46:24 PDT
Created attachment 767718 [details] [diff] [review]
One line fix

Okay, here it goes
Comment 3 Phoenix 2013-06-26 06:03:47 PDT
Comment on attachment 767718 [details] [diff] [review]
One line fix

Changed reviewer per Ratty's comment in irc
Comment 4 neil@parkwaycc.co.uk 2013-06-26 07:10:35 PDT
(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 2013-06-27 04:40:40 PDT
> 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 2013-06-27 14:10:13 PDT
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 2013-06-28 04:49:22 PDT
Created attachment 768880 [details] [diff] [review]
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 2013-06-28 07:33:37 PDT
> +  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 2013-06-28 08:05:51 PDT
On other hand, around Services.prefs in this files there are isles of lets :)
Comment 10 Philip Chee 2013-06-29 04:27:25 PDT
> 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 2013-07-03 14:46:28 PDT
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 neil@parkwaycc.co.uk 2013-07-03 15:56:38 PDT
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 2013-07-04 04:13:06 PDT
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 neil@parkwaycc.co.uk 2013-07-04 05:04:06 PDT
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 16 Philip Chee 2013-07-08 09:36:17 PDT
*** Bug 890911 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.