717384 - Move the Source Editor to toolkit

Status: RESOLVED WONTFIX

(DevTools :: Source Editor, enhancement, P4)

Description

[Mihai Sucan [:msucan]]

Some people have expressed interest in reusing the Source Editor in different applications, outside of Firefox. They asked that the Source Editor moves to toolkit.

I believe we should consider this possibility once the Source Editor is mature enough.

(please add comments on requirements for this to happen, and blockers we need to fix. thank you!)

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

As it currently looks like, Firebug 1.10 will use/require SourceEditor, and without this bug getting fixed or a fallback implemented on Firebug's side, this will make it impossible to use Firebug with SeaMonkey.

Cf. <http://code.google.com/p/fbug/issues/detail?id=5219>

Comment 2

[Rob Campbell [:rc] (:robcee)]

Moving to Source Editor component.

Filter on CHELICERAE.

Comment 3

[Alex Vincent [:WeirdAl]]

I see the bugs this depends on have been marked fixed.  What prevents us from doing a wholesale move from browser/devtools/sourceeditor to toolkit/devtools/sourceeditor ?

I note that a lot of the references go directly to "resource:///modules/source-editor.jsm", which is wrong. They should point to resource://gre/modules/source-editor.jsm after the move.  (Technically, they should point to resource://app/... now.)

I can probably start crafting a patch this weekend.

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: patch, minimally tested

Or maybe I can do it sooner... :)

Comment 5

[Mihai Sucan [:msucan]]

Comment on attachment 738711 [details] [diff] [review]
patch, minimally tested

Review of attachment 738711 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good. Nothing obvious holds us away from moving the source editor to toolkit. Your patch is very much welcome. Thank you!

Before we can do the move all of the source editor tests need to become mochitest-chrome tests. This means you can't open new tabs or windows. This can be easily changed: mostly mechanical conversion of the existing js tests into xul tests. Can you please do this? Once you have converted one test file please ping me on IRC so I can check how it is before you do the rest of the tests. Thanks!

::: browser/devtools/debugger/debugger-controller.js
@@ +20,5 @@
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>  Cu.import("resource://gre/modules/devtools/dbg-server.jsm");
>  Cu.import("resource://gre/modules/devtools/dbg-client.jsm");
>  Cu.import("resource://gre/modules/commonjs/sdk/core/promise.js");
> +Cu.import("resource://gre/modules/source-editor.jsm");

Please move source-editor.jsm into resource://gre/modules/devtools.

::: browser/devtools/sourceeditor/source-editor.jsm
@@ +19,5 @@
>  try {
>    if (component == "ui") {
>      throw new Error("The ui editor component is not available.");
>    }
> +  Cu.import("resource://gre/modules/source-editor-" + component + ".jsm", obj);

Ditto for the rest of source editor jsm files.

Comment 6

[Alex Vincent [:WeirdAl]]

Comment on attachment 738711 [details] [diff] [review]
patch, minimally tested

This patch wasn't even close to complete:  I forgot to move locale & skin files.

Comment 7

[Alex Vincent [:WeirdAl]]

Attachment: patch with locales & themes moved

Rewriting of tests is forthcoming.

Comment 8

[Daniel Brooks [:db48x]]

Attachment: partially converted tests, wip

Got held up on a problem in Orion; it tries to clear the content of its inner iframe (using document.open) only to find that this is dissallowed. I believe that the outer iframe should be type=content so that the entire editor is content instead of half-chrome-half-content.

Comment 9

[Alex Vincent [:WeirdAl]]

Daniel, do you want to regenerate that patch using diff --git, to show repo moves? :-)  The patch is rather large without that...

Comment 10

[Daniel Brooks [:db48x]]

Attachment: wip, updated

Comment 11

[Alex Vincent [:WeirdAl]]

Attachment: wip, updated (patches folded into one)

Well, tests are failing early, due to exceptions as Daniel reports in comment 8.  Mihai, you can probably identify the problems and direct us on the fixes fairly easily.  Suggestions are welcome.

Comment 12

[Mihai Sucan [:msucan]]

Alex and Daniel: thank you for your patch. I believe you saw that the devtools team is planning to move all tools into their own folder and that means we'll move the source editor again. Should we do this move here and now?

The split between toolkit/ and browser/ for devtools is becoming a problem for us and we would like to solve the problem for all tools at once.

Comment 13

[Alex Vincent [:WeirdAl]]

(In reply to Mihai Sucan [:msucan] from comment #12)
> Alex and Daniel: thank you for your patch. I believe you saw that the
> devtools team is planning to move all tools into their own folder and that
> means we'll move the source editor again. Should we do this move here and
> now?

No, that makes little sense.  Also, on irc, there was some discussion of replacing Orion with codemirror (but no decision).  It sounds like we're jumping the gun just a little bit.

Comment 14

[Mihai Sucan [:msucan]]

Comment on attachment 750475 [details] [diff] [review]
wip, updated (patches folded into one)

Canceling feedback request then. Thanks!

Comment 15

[Gabriel Luong [:gl] (ΦωΦ)]

I haven't seen any activity in this bug or heard any interest about moving the source editor into toolkit. I assume this is safe to mark as WONTFIX. Please reopen if these assumptions are wrong.