Move Editor startup/shutdown functions to editingOverlay

RESOLVED FIXED in seamonkey2.9

Status

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

Tracking

Trunk
seamonkey2.9
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.9 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 568172 [details] [diff] [review]
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
Attachment #568172 - Flags: review?(neil)
(Assignee)

Comment 1

7 years ago
Created attachment 568241 [details] [diff] [review]
Move startup/shutdown functions (unbitrotted)

Unbitrotted taking into account changes from bug 682580
Attachment #568172 - Attachment is obsolete: true
Attachment #568172 - Flags: review?(neil)
Attachment #568241 - Flags: review?(neil)

Comment 2

7 years ago
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?]
(Assignee)

Comment 3

7 years ago
Created attachment 581699 [details] [diff] [review]
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?
Attachment #568241 - Attachment is obsolete: true
Attachment #568241 - Flags: review?(neil)
Attachment #581699 - Flags: review?(neil)

Comment 4

7 years ago
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)
Attachment #581699 - Flags: review?(neil) → review+
(Assignee)

Comment 5

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

Fixed typo and reformatted docshell code to improve readability (hopefully)
Carrying forward r+
Attachment #581699 - Attachment is obsolete: true
Attachment #581963 - Flags: review+
(Assignee)

Comment 6

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

Requesting additional review on shared code removal.
Attachment #581963 - Flags: review?(mbanner)
Attachment #581963 - Flags: review?(mbanner) → review+
(Assignee)

Comment 7

7 years ago
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
Attachment #581963 - Attachment description: Move startup/shutdown functions v2.1 → Move startup/shutdown functions v2.1 [Checked in: Comment 7]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.9
(Assignee)

Updated

7 years ago
status-seamonkey2.9: --- → fixed
You need to log in before you can comment on or make changes to this bug.