Closed Bug 887183 Opened 7 years ago Closed 7 years ago

"GetPromptService is not defined" errors in Error Console

Categories

(SeaMonkey :: Composer, defect)

defect
Not set

Tracking

(seamonkey2.19 affected, seamonkey2.20 fixed, seamonkey2.21 fixed, seamonkey2.22 fixed)

RESOLVED FIXED
seamonkey2.22
Tracking Status
seamonkey2.19 --- affected
seamonkey2.20 --- fixed
seamonkey2.21 --- fixed
seamonkey2.22 --- fixed

People

(Reporter: pppx, Assigned: pppx)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
>  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)
Attached patch One line fix (obsolete) — Splinter Review
Okay, here it goes
Assignee: nobody → pppx
Status: NEW → ASSIGNED
Attachment #767718 - Flags: review?(neil)
Comment on attachment 767718 [details] [diff] [review]
One line fix

Changed reviewer per Ratty's comment in irc
Attachment #767718 - Flags: review?(neil) → review?(iann_bugzilla)
(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
> 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 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.
Attachment #767718 - Flags: review?(iann_bugzilla) → review+
Done, also replaced some vars with lets as in Bug 795158, please check
Attachment #767718 - Attachment is obsolete: true
Attachment #768880 - Flags: review?(iann_bugzilla)
> +  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".
On other hand, around Services.prefs in this files there are isles of lets :)
> On other hand, around Services.prefs in this files there are isles of lets :)
oh well, if that's the case...
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.
Attachment #768880 - Flags: review?(iann_bugzilla) → review+
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.
Keywords: checkin-needed
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.
Attachment #768880 - Attachment description: Aligned with Bug 795158 style → Aligned with Bug 795158 style [check-in comment 13]
Attachment #768880 - Flags: approval-comm-beta?
Attachment #768880 - Flags: approval-comm-aurora?
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).
Attachment #768880 - Flags: approval-comm-beta?
Attachment #768880 - Flags: approval-comm-beta+
Attachment #768880 - Flags: approval-comm-aurora?
Attachment #768880 - Flags: approval-comm-aurora+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 890911
You need to log in before you can comment on or make changes to this bug.