207315 - Chop up editor.js

Status: VERIFIED DUPLICATE of bug 224578

(Firefox :: Settings UI, defect)

Description

[alanjstr]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030527 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030527 Mozilla Firebird/0.6

editor.js and mailnews.js should be removed from the build.  

Reproducible: Always

Steps to Reproduce:

Comment 1

[alanjstr]

*** This bug has been marked as a duplicate of 171082 ***

Comment 2

[alanjstr]

Since Bug 171082 seems to be a Tracker, this will cover an individual component.

Comment 3

[David P James]

Confirming. According to bug 171082 comment #36 there are some editor.js prefs
required by Firebird.

Comment 4

[alanjstr]

Some stuff should be moved to all.js
See bug 171082 comment 36

Comment 5

[Mike Connor [:mconnor]]

taking QA contact, sorry about the bugspam

Comment 6

[alanjstr]

I've removed editor.js from my directory.  The prefs no longer show up in
about:config.  Nothing has crashed.  0.6.1 milestone.  I think this can just be
removed from the tree.

Comment 7

[alanjstr]

editor.js needs to be moved to the composer directory directory.

Comment 8

[alanjstr]

This is fixed in the Win32 installer, but not the zips.  Maybe the installer has
it inside but doesn't know to unpack it?

Comment 9

[Steffen Wilberg]

Attachment: patch to remove editor.js and mailnews.js

Include editor.js and mailnews.js only if we're not building Phoenix.

If you want to try this: Remove these files from mozilla/dist/bin if you don't
do "make clean" before building.

Comment 10

[Steffen Wilberg]

Comment on attachment 135471 [details] [diff] [review]
patch to remove editor.js and mailnews.js

Brian, review?
Firebird compiles fine. I didn't try SeaMonkey.

I don't know what "GARBAGE" is about, so I'm not sure whether that should be
changed.

Comment 11

[Steffen Wilberg]

Another option would be to check for MOZ_MAIL_NEWS, MOZ_LDAP_XPCOM and MOZ_COMPOSER.

Comment 12

[Steffen Wilberg]

Taking and targetting.

Comment 13

[Brian Ryner (not reading)]

Comment on attachment 135471 [details] [diff] [review]
patch to remove editor.js and mailnews.js

Some prefs in editor.js are needed in Firebird for Midas (content-editable). 
Not sure about mailnews.js.  Also, this is incorrect usage of $(addprefix) in
GARBAGE... you want:

GARBAGE += $(addprefix $(DIST)/bin/defaults/pref/, mailnews.js editor.js)

or just leave it be, extra stuff in GARBAGE doesn't hurt anything.

Comment 14

[alanjstr]

What from editor.js is needed for midas?  Is midas built in now?  why are these
settings in editor.js and not midas.js?  

Comment 15

[Mike Connor [:mconnor]]

*** Bug 228457 has been marked as a duplicate of this bug. ***

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

-> 0.9

Comment 17

[Steffen Wilberg]

Attachment: also remove SeaMonkey's all.js; add pref

I've tested rich text editing (midas) with
http://www.mozilla.org/editor/midasdemo/ and
http://www.kevinroth.com/rte/demo.htm

It works pretty well without editor.js. We only need to add the pref
"editor.use_css" and set it to true like in editor.js, in order to make text
highlighting ("hilitecolor") work. I've added that one to all.js. I also
ifdef'ed Seamonkey's all.js, because that one gets replaced by Firebird's
all.js anyway.
I don't think we need mailnews.js. Mailto links, send page and send image work
fine without it.
I also corrcted the garbage stuff.

Comment 18

[Steffen Wilberg]

Comment on attachment 138815 [details] [diff] [review]
also remove SeaMonkey's all.js; add pref 

Brian, please have a look.

>+GARBAGE     += $(addprefix $(DIST)/bin/defaults/pref/, all.js mailnews.js editor.js)
Argh. Please replace the spaces with two tabs. :)

Comment 19

[Steffen Wilberg]

Bug 207317 is about chopping mailnews.js. It depends on bug 224339, which is
already fixed. Blocking bug 207317 since my patch fixes that as well.

Comment 20

[Brian Ryner (not reading)]

I would say we need any prefs that are checked from the source code in
editor/libeditor, editor/composer, editor/txtsvc, and editor/txmgr to be present
in Firebird.

Running
$ grep -ri "pref.*editor\." composer libeditor txmgr txtsvc

gives this list:

libeditor/base/nsEditor.cpp:    (void) prefBranch->GetIntPref("editor.htmlWrapCo
lumn", aWrapColumn);
libeditor/html/nsHTMLAbsPosition.cpp:    res = prefBranch->GetIntPref("editor.po
sitioning.offset", &positioningOffset);
libeditor/html/nsHTMLCSSUtils.cpp:    result = prefBranch->GetBoolPref("editor.u
se_css", &mIsCSSPrefChecked);
libeditor/html/nsHTMLCSSUtils.cpp:    result = prefBranch->GetBoolPref("editor.u
se_custom_colors", &useCustomColors);
libeditor/html/nsHTMLCSSUtils.cpp:      result = prefBranch->GetCharPref("editor
.background_color",
libeditor/html/nsHTMLCSSUtils.cpp:    result = prefBranch->GetCharPref("editor.c
ss.default_length_unit",
libeditor/html/nsHTMLDataTransfer.cpp:    prefBranch->GetBoolPref("editor.quotes
Preformatted", &quotesInPre);
libeditor/html/nsHTMLEditRules.cpp:  res = prefBranch->GetCharPref("editor.html.
typing.returnInEmptyListItemClosesList",
libeditor/html/nsHTMLObjectResizer.cpp:    result = prefBranch->GetBoolPref("edi
tor.resizing.preserve_ratio", &preserveRatio);
libeditor/text/nsTextEditRules.cpp:    res = prefBranch->GetIntPref("editor.sing
leLine.pasteNewlines",

Comment 21

[Steffen Wilberg]

Brian, do we really need all those prefs? Even those set to false or 0?
editor.html.typing.returnInEmptyListItemClosesList is not part of editor.js.
Even if we do need the rest of them, 9 prefs are better than some 65.
Should we add them to all.js, or create a new editor.js in moz/browser?

Comment 22

[Brian Ryner (not reading)]

Ideally, I'd like to move towards something like this.  Eliminate all.js, and have:

everyone:
gecko.js  (gecko default prefs, including core editor/midas prefs)

seamonkey:
composer.js (composer-specific default prefs)
mailnews.js (seamonkey mail/news default prefs)
navigator.js (seamonkey browser default prefs)

firebird:
firebird.js (firebird browser default prefs)

thunderbird:
thunderbird.js (thunderbird default prefs)

I'm not sure how easy this is to implement, and we definitely need the prefs
files to be read in an order such that the app prefs file can override anything
else.

But I think the separation here is a good start.  I would put any pref in that
list into all.js, even the ones that are false or 0.  No need for a separate
editor.js for firebird at this point.

Comment 23

[alanjstr]

Bryner-

Sounds like you want bug 62755 to be fixed, then.

Comment 24

[Steffen Wilberg]

Attachment: add the editor prefs we need to all.js

Same as the last patch, but also adds the editor prefs mentioned in comment 20
from modules/libpref/scr/init/editor.js to browser's all.js, as suggested in
comment 22. The only exception is
editor.html.typing.returnInEmptyListItemClosesList, because that pref is not
set anywhere. The order of the prefs is the same as in editor.js.

Comment 25

[Steffen Wilberg]

Attachment: also fix indenting in makefile.in

Grr, I had to reconfigure my text editor to insert tabs instead of spaces.
Sorry for spamming.

Comment 26

[Steffen Wilberg]

Comment on attachment 139012 [details] [diff] [review]
also fix indenting in makefile.in

Because of bug 224578,	editor.js (now composer.js) and mailnews.js are no
longer part of Firebird.

The midas prefs I wanted to move with this patch are now part of the big
greprefs/all.js (as well as the platform specifc pref files like winpref.js).

This patch is completely obsolete therefore.

Comment 27

[alanjstr]

So is this bug considered FIXED?

Comment 28

[Steffen Wilberg]

Yes, it's fixed! But since bug 224578 did the job, I'm duping this bug to that
one. Also removing some dependencies.

*** This bug has been marked as a duplicate of 224578 ***

Comment 29

[Simon Paquet [:sipaq]]

verified duplicate

Comment 30

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change