695842 - Move Editor startup/shutdown functions to editingOverlay

Status: RESOLVED FIXED

(SeaMonkey :: Composer, defect)

Description

[Ian Neal]

Attachment: Move startup/shutdown functions

editor.js currently has a number of functions which are only used by editor.xul and debugQATextEditorShell.xul
This patch:
* moves EditorOnLoad, TextEditorOnLoad, EditorShutdown, EditorStartup and EditorCanClose from editor.js to editingOverlay.js
* merges TextEditorOnLoad and EditorStartup into EditorOnLoad

Comment 1

[Ian Neal]

Attachment: Move startup/shutdown functions (unbitrotted)

Unbitrotted taking into account changes from bug 682580

Comment 2

[[e-mail]]

Comment on attachment 568241 [details] [diff] [review]
Move startup/shutdown functions (unbitrotted)

>+function EditorOnLoad(aHTMLEditor)
...
>+  var is_HTMLEditor = IsHTMLEditor();
>+  if (is_HTMLEditor)
I don't think you need both of these. One alternative would be to move this block to the end of the function. Or, since the text editor is a subset of the editor, you might write something like this:
function EditorOnLoad()
{
  TextEditorOnLoad();

  gContentWindowDeck = document.getElementById("ContentWindowDeck");
  gFormatToolbar = document.getElementById("FormatToolbar");
  gViewFormatToolbar = document.getElementById("viewFormatToolbar");

  // Initialize our source text <editor>
  // etc.
}

>+  // Startup also used by other editor users, such as Message Composer
>+  EditorSharedStartup();
[Since both web composer and text editor are using this function, Message Compose is now the sole remaining other editor user ;-) ]

>+  gCSSPrefListener = new nsPrefListener(kUseCssPref);
>+  gReturnInParagraphPrefListener = new nsPrefListener(kCRInParagraphsPref);
[These probably do nothing in text editors, but it makes shutdown easier.]

>+  // hide Highlight button if we are in an HTML editor with CSS mode off
[As does this, so it could be moved to the composer-only block.]

>+  // Get url for editor content and load it. The editor gets instantiated by
>+  // the edittingSession when the URL has finished loading.
[editing session?]

Comment 3

[Ian Neal]

Attachment: Move startup/shutdown functions v2

Changes since last version:
* Recreated EditorStartup for the code shared between web composer and text editor (did not seem to be a straight forward way of just having TextEditorOnLoad being called by EditorOnLoad).
* Fixes missing variable declaration from Bug 102275

By the looks of it http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.xul#154 defaults the url to about:blank, so added that to the code (though works both with and without that change).
Not sure why we have lots of broadcasters for args:
http://mxr.mozilla.org/comm-central/search?string=broadcaster+id%3D%22args%22&case=1&find=editor&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
Should those be removed as part of another bug?

Comment 4

[[e-mail]]

Comment on attachment 581699 [details] [diff] [review]
Move startup/shutdown functions v2

>+  var root = ds.QueryInterface(Components.interfaces.nsIDocShellTreeItem)
>+               .rootTreeItem.QueryInterface(Components.interfaces.nsIDocShell);
>+
>+  root.QueryInterface(Components.interfaces.nsIDocShell).appType =
>+    Components.interfaces.nsIDocShell.APP_TYPE_EDITOR;
It looks as if you just cut and pasted everything but even so this looks odd ;-)

>+  // Set up our global prefs object.
>+  GetPrefsService();
[Bah, editor hasn't switched to Services.prefs yet?]

>+  // EditorSharedStartup also used by Message Composer.
Message Compose (no r)

Comment 5

[Ian Neal]

Attachment: Move startup/shutdown functions v2.1 [Checked in: Comment 7]

Fixed typo and reformatted docshell code to improve readability (hopefully)
Carrying forward r+

Comment 6

[Ian Neal]

Comment on attachment 581963 [details] [diff] [review]
Move startup/shutdown functions v2.1 [Checked in: Comment 7]

Requesting additional review on shared code removal.

Comment 7

[Ian Neal]

Comment on attachment 581963 [details] [diff] [review]
Move startup/shutdown functions v2.1 [Checked in: Comment 7]

http://hg.mozilla.org/comm-central/rev/3e9df5c47185