106111 - the editor lib needs ability to be built w/ out HTML composition

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Judson Valeski]

This will allow folks just needing plain text form widgets to get a slimmed down
lib.

Comment 1

[Alec Flett]

it's already built by default, actually! embedding just needs to update
basebrowser-* to use texteditor.dll instead of editor.dll

Comment 2

[Akkana Peck]

This was filed on Linux, and we don't build no texteditor.dll there.

Comment 3

[Akkana Peck]

I have a patch.  Build with --enable-plaintext-editor-only.  In my debug tree,
this gets you a libeditor.so of 3.2M rather than 5.4M (presumably much smaller
numbers in optimized).

Comment 4

[Akkana Peck]

Attachment: Patch: switch on --enable-plaintext-editor-only

Comment 5

[Alec Flett]

cc seawood for configure issues. Wondering if we just want a generic
--minimal-embedding or something that turns this one on.

Comment 6

[Chris McAfee]

r=mcafee, I helped akkana come up with these changes.

Comment 7

[Judson Valeski]

This starts wading into the water of having a #ifdef wandering it's way all over
the code (ratbert style) which requires a fair amount of overhead (at the
developer level (maintianing two trees at times), and the tbox level (one
"regular tbox" and one "minimal" tbox). 

How would --minimal-build or --embedding-build tie into the SDK stuff you're
currently hammering on seawood? This is obviously the build flag model.
Presumably there would be a set of #defines (NO_XUL being another) that
--embedding-build would be flipped.

Comment 8

[Akkana Peck]

I should have asked: would it be easier if I set unix up to build both regular
and text-only libraries every time?  I can do it that way if anyone thinks it's
better.

Cc'ing kin for comment, and also for sr if he decides that this way (build only
one library at a time) is better.

Comment 9

[hacker formerly known as [e-mail]]

Comment on attachment 54564 [details] [diff] [review]
Patch: switch on --enable-plaintext-editor-only

Turning on just the plaintext editor shouldn't cause a tree rebuild since it's all localized to the editor.  Follow the HAVE_MOVEMAIL example and just add the DEFINE to the makefiles where it's needed

Comment 10

[Judson Valeski]

we run into the reverse problem if we do that (the core mozilla builds have to
pick one for final distribution).

Comment 11

[hacker formerly known as [e-mail]]

Comment on attachment 54564 [details] [diff] [review]
Patch: switch on --enable-plaintext-editor-only

*sigh* Ignore my previous comment.  Our dependency system can't handle what I proposed so we'll have to live with the rebuilds until I can fix it.

Comment 12

[hacker formerly known as [e-mail]]

Off-topic for this bug, but you asked. :) The SDK work is, IMO, completely
separate from the --embedding-build flag.  As I see it, the SDK work would be to
carve out the core set of modules from the Mozilla-browser build that could be
used to build an app using gecko. (And then build Mozilla using that SDK.)  This
--embedding-build flag seems to serve to further trim off features from those
modules.  The point at where those 2 efforts would merge would be when you can
trim off features in such a way as to allow the trimmed feature to be its own
entity so that no extra defines are needed. Then the SDK could pick up the
original feature that it wanted w/o the feature that was left on the cutting
room floor.

Comment 13

[kinmoz]

To answer akk's question:

I'd like to see us build both to prevent bit rot and help catch situations where
people are introducing bad dependencies in the editor core code.

Of course this can't happen until we move cmanske's SetDocTitleTxn out of base
and into the html dir so that we don't have to use ifdefs in the base factory code.

Comment 14

[Alec Flett]

I like that plan too...

is there a bug # on that issue?
It looks like we already do this on windows though - is the library still
functional on windows?

Comment 15

[kinmoz]

No bug filed on the transaction issue, but it's glaringly obvious if you try to
build both the stripped down and full blown libraries using the same factory
object file because you'll get undefined symbols during link time.

The stripped down library should still work on win32 ... though I haven't tried
it in a month or so.

Comment 16

[Akkana Peck]

Attachment: new patch: Don't set the -D define at all

Comment 17

[Akkana Peck]

Kin and I poked around and discovered that Windows wasn't setting the -D at all,
and it worked.  So for consistency, I'm no longer setting it either (which also
addresses Chris' objection even though he retracted the objection; I thought he
had a good point even if it doesn't matter right now).

Comment 18

[Alec Flett]

Comment on attachment 54970 [details] [diff] [review]
new patch: Don't set the -D define at all

on windows, it actually builds both libraries by spawning an additional $(MAKE) with MOZ_BUiLD_PLAINTEXT_EDITOR_ONLY

http://lxr.mozilla.org/seamonkey/source/editor/libeditor/build/makefile.win#150

Comment 19

[Akkana Peck]

Targeting 0.9.6 -- all this needs is an sr.  We can address the "build both"
issue later (though I'll work on it in the meantime).

Comment 20

[kinmoz]

Comment on attachment 54970 [details] [diff] [review]
new patch: Don't set the -D define at all

sr=[e-mail]

Comment 21

[Akkana Peck]

Fix checked in.  I'll leave the bug open for the problem of building both
libraries by default, as Windows does.

Comment 22

[Akkana Peck]

Building both libraries can wait 'til the next milestone.

Comment 23

[Akkana Peck]

Filed bug 108263 on building both, rather than morphing this bug.  Marking
fixed.

Comment 24

[Akkana Peck]

I'll go ahead and verify this, though it would be nice if someone from embedding
who actually needs this functionality cross-checked it.