69995 - [minimo] make stuff work with #ifdef MOZ_XUL turned *off* (--disable-xul)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Chris Waterson]

To satiate short term needs for our ``minimal HTML embed build'' consumers, 
normalize #ifdef INCLUDE_XUL and #ifdef MOZ_XUL, and make stuff generally work 
with this turned off.

Comment 1

[Chris Waterson]

This should allow us to jettison chrome.dll and rdf.dll from the 
``minimal'' build, too.

Comment 2

[David Hyatt]

I think we need a better ifdef name then MOZ_XUL if we're dumping RDF also...
maybe MOZ_EMBED or MOZ_LITE.

Comment 3

[Judson Valeski]

note: global history uses rdf to persist the history state. Although global 
history is optional (w/ another mechanism for visited link coloring (pushing 
that off onto the embeddor)), it's obviously going to be used most of the time, 
and will subsequently drag in rdf (unless we persist global history in another 
format).

Comment 4

[Chris Waterson]

hyatt: the MOZ_XUL #ifdef is primarily a layout-ism (like MOZ_SVG and 
MOZ_MATHML). It means, ``we're too stupid and lazy to figure out how to properly 
factor frames n' stuff out into DLLs right now.''

I envision that the ``ultralite'' embed build will turn off MOZ_XUL, as well as 
a boatload of other components (like rdf, chrome, global history, bookmarks, 
etc.)

jud: We should probably write a ``lite global history component'' for the 
purpose of link coloring (or tell embedders that they need to implement the 
global history interface). Our current implementation pulls in a bunch of other 
stuff (mork, at a minimum) that probably are just inappropriate. (FYI, global 
history doesn't use RDF to persist history state, it uses Mork for that.)

Comment 5

[Judson Valeski]

radha posted a mechanism for embeddors to get link coloring w/ out global 
history to .embedding the other day. Basically, they need to impl a "visted()" 
method off of nsIWebBrowserChrome.

If the embeddor doesn't want our rdf'ified, mork'ified, global history impl, 
they can do whatever they want. Though arguements for a lighter version of our 
own can still be made.

Comment 6

[Alec Flett]

by the way - global history does NOT use rdf to persist it's state, it uses
mork. However, at runtime, it behaves as an RDF datasource, in order to display
the history data in a user interface.

Comment 7

[Alec Flett]

oops, waterson already said that :)

we've talked about breaking global history into two interfaces, one for
embedders so they could cheaply re-implement global history without mork, and
one for mozilla-the-browser...

Comment 8

[cls]

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

Comment 9

[cls]

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

Comment 10

[Carsten Menke]

Same Problem here: --disable-xul stops compiling here:

######################### OUTPUT
#######################################nsCSSParser.cpp:2271: parse error before `{'
nsCSSParser.cpp: At top level:
nsCSSParser.cpp:2280: warning: ANSI C++ forbids declaration `aParsingStatus'
with no type
nsCSSParser.cpp:2282: parse error before `if'
nsCSSParser.cpp:2308: warning: ANSI C++ forbids declaration `newSel' with no type
nsCSSParser.cpp:2308: conflicting types for `int newSel'
nsCSSParser.cpp:2281: previous declaration as `struct nsCSSSelector * newSel'
nsCSSParser.cpp:2308: warning: initialization to `int' from `nsCSSSelector *'
lacks a cast
nsCSSParser.cpp:2309: `aSelector' was not declared in this scope
nsCSSParser.cpp:2310: parse error before `while'
nsCSSParser.cpp:2315: syntax error before `->'
nsCSSParser.cpp:2316: `aDataMask' was not declared in this scope
nsCSSParser.cpp:2316: invalid type argument of `unary *'
nsCSSParser.cpp:2316: `aErrorCode' was not declared in this scope
nsCSSParser.cpp:2316: warning: ANSI C++ forbids declaration
`ParseTypeOrUniversalSelector' with no type
nsCSSParser.cpp:2316: warning: initializer list being treated as compound expression
nsCSSParser.cpp:2317: parse error before `}'
nsCSSParser.cpp:2325: warning: ANSI C++ forbids declaration `aParsingStatus'
with no type
nsCSSParser.cpp:2325: redefinition of `int aParsingStatus'
nsCSSParser.cpp:2280: `int aParsingStatus' previously defined here
nsCSSParser.cpp:2326: parse error before `}'
nsCSSParser.cpp:2330: warning: ANSI C++ forbids declaration `aParsingStatus'
with no type
nsCSSParser.cpp:2330: redefinition of `int aParsingStatus'
nsCSSParser.cpp:2325: `int aParsingStatus' previously defined here
nsCSSParser.cpp:2331: parse error before `}'
nsCSSParser.cpp:2537: confused by earlier errors, bailing out
cpp0: output pipe has been closed
make[6]: *** [nsCSSParser.o] Error 1
make[6]: Leaving directory `/usr/local/src/net/mozilla/content/html/style/src'
make[5]: *** [libs] Error 2
make[5]: Leaving directory `/usr/local/src/net/mozilla/content/html/style'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/usr/local/src/net/mozilla/content/html'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/usr/local/src/net/mozilla/content'
make[2]: *** [tier_9] Error 2
make[2]: Leaving directory `/usr/local/src/net/mozilla'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/usr/local/src/net/mozilla'
make: *** [build] Error 2

######################## END OUTPUT ########################

Build System: Linux 2.4.8-ac7 gcc-2.95.3 glib-2.2.2

Comment 11

[Daniel Switkin]

Would someone please add embed to the keywords, I lack the authority.

Comment 12

[Mike Gerdts]

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

Comment 13

[Mike Gerdts]

Bug 134569 has a patch for nsCSSParser.cpp failure reported in comment 10.

Comment 14

[Mike Gerdts]

Attachment: Patch to allow nsXBLWindowHandler.cpp to compile --without-xul

Comment 15

[Mike Gerdts]

Build now stops at:

/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp: In member 
   function `virtual nsresult 
   nsCSSFrameConstructor::ConstructRootFrame(nsIPresShell*, nsIPresContext*, 
   nsIContent*, nsIFrame*&)':
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3699:
`NS_NewScrollPortFrame'
   undeclared (first use this function)
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3699: (Each 
   undeclared identifier is reported only once for each function it appears 
   in.)
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp: At global 
   scope:
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3725:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3738:
syntax    error before `.' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3753:
`aPresShell'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3753:
`pageFrame'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3753: ISO 
   C++ forbids declaration of `NS_NewPageFrame' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3753: `int 
   NS_NewPageFrame' redeclared as different kind of symbol
/jumpstart/mozilla/layout/html/base/src/nsHTMLParts.h:171: previous
declaration    of `nsresult NS_NewPageFrame(nsIPresShell*, nsIFrame**)'
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3753:
initializer
   list being treated as compound expression
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3757: ISO 
   C++ forbids declaration of `mFixedContainingBlock' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3757:
`pageFrame'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3763:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3767:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3769:
`aPresContext'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3769:
`pageFrame'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3770: ISO 
   C++ forbids declaration of `CreateViewForFrame' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3770: `int 
   nsHTMLContainerFrame::CreateViewForFrame' is not a static member of `class 
   nsHTMLContainerFrame'
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3770:
initializer
   list being treated as compound expression
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3772:
`aPresShell'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3772: ISO 
   C++ forbids declaration of `NS_NewPageContentFrame' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3772: `int 
   NS_NewPageContentFrame' redeclared as different kind of symbol
/jumpstart/mozilla/layout/html/base/src/nsHTMLParts.h:172: previous
declaration    of `nsresult NS_NewPageContentFrame(nsIPresShell*, nsIFrame**)'
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3772:
initializer
   list being treated as compound expression
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3775:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3779:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3780:
`aPresContext'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3781: ISO 
   C++ forbids declaration of `CreateViewForFrame' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3781: `int 
   nsHTMLContainerFrame::CreateViewForFrame' is not a static member of `class 
   nsHTMLContainerFrame'
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3781:
initializer
   list being treated as compound expression
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3784: ISO 
   C++ forbids declaration of `mDocElementContainingBlock' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3784:
cannot    convert `nsIFrame*' to `int' in initialization
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3785: ISO 
   C++ forbids declaration of `mFixedContainingBlock' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3785:
redefinition
   of `int mFixedContainingBlock'
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3757: `int 
   mFixedContainingBlock' previously defined here
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3785:
cannot    convert `nsIFrame*' to `int' in initialization
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3788:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3791:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3795:
syntax    error before `->' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3797: ISO 
   C++ forbids declaration of `aNewFrame' with no type
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3797:
`viewportFrame'
   was not declared in this scope
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:3801: parse 
   error before `return'
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp: In member 
   function `nsresult 
   nsCSSFrameConstructor::CreateAnonymousFrames(nsIPresShell*,
nsIPresContext*,    nsIAtom*, nsFrameConstructorState&, nsIContent*, nsIFrame*,
nsFrameItems&, 
   int)':
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:5016: parse 
   error before `)' token
/jumpstart/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:5020:
confused by earlier errors, bailing out
gmake[1]: *** [nsCSSFrameConstructor.o] Error 1
gmake[1]: Leaving directory `/var/tmp/mozilla-0.9.9/layout/html/style/src'
gmake: *** [all] Error 2

Comment 16

[Alec Flett]

as of right now, my understanding is that we are not going to try to get this
working - meaning that
1) XUL is already too intertwined into Gecko to make this worth the effort
2) in the distant future, XBL-defined form controls will require XUL, (and we'll
throw out the existing C++-defined HTML form controls) so long term, XUL will be
required to render any kind of UI

Comment 17

[Brian Ryner (not reading)]

-> me.

Comment 18

[Brian Ryner (not reading)]

Attachment: *rough* patch, probably not ready to go in

Use At Your Own Risk, but this works for me with TestGtkEmbed.

Comment 19

[Brian Ryner (not reading)]

Note that one obstacle to getting rid of all of the XUL frame code is the fact
that nsTextControlFrame inherits from nsStackFrame, which inherits from
nsBoxFrame and requires StackLayout, which inherits from BoxLayout.  If we
wanted to remove Box entirely, we would have to break that dependency as well as
switch to native scrollbars.

Comment 20

[Brian Ryner (not reading)]

Attachment: revised patch

Comment 21

[Brian Ryner (not reading)]

Attachment: updated patch

Changes:
- merged to the trunk
- fixed an #ifdef mistake in XBL
- made configure automatically remove extensions that depend on XUL if you
specify --disable-xul
- misc. cleanup

Comment 22

[Alec Flett]

Comment on attachment 118698 [details] [diff] [review]
updated patch

this is really great. thanks for tackling this (Again!)
sr=alecf
very cool to see that nsDialogParamBlock can be yanked :)

Comment 23

[Alec Flett]

hey, so what's up with this? Until this lands, we can't do --disable-xul for
minimo. The patch has an sr=alecf, still needs r=dbaron...

Comment 24

[Alec Flett]

looks like this patch has bit-rotted slightly due to the accessibility reorg.
most of the stuff in mozilla/accessible is dead now :(

Comment 25

[Brian Ryner (not reading)]

Comment on attachment 118698 [details] [diff] [review]
updated patch

If dbaron doesn't have time to look at this, could you Boris?

Comment 26

[Alec Flett]

I recently applied this patch and built - a few issues:
- since this patch was made, we also need to add MOZ_XUL stuff in dom/base/build
in order to handle the nsIXULPrototypeCache stuff
- the accessibility stuff has been completely reorg'ed and I'm not sure what to
do there... my builds are also --disable-accessibility though, so I am able to
finish the build without dealing with that. (a quick fix would be to make
configure.in disable accessibility automatically when --disable-xul was used)

Comment 27

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 118698 [details] [diff] [review]
updated patch

>Index: content/xbl/src/nsXBLBinding.cpp
>+      anonymous->SetDocument(nsnull, PR_TRUE, PR_TRUE); // Kill it.
>+
>+#ifdef MOZ_XUL
>       // To make XUL templates work (and other XUL-specific stuff),
>       // we'll need to notify it using its add & remove APIs. Grab the
>       // interface now...
>       nsCOMPtr<nsIXULDocument> xuldoc(do_QueryInterface(aOldDocument));
>-
>-      anonymous->SetDocument(nsnull, PR_TRUE, PR_TRUE); // Kill it.
>       if (xuldoc)
>         xuldoc->RemoveSubtreeFromDocument(anonymous);

Are we sure that anonymous is not holding the rest ref to aDocument by any
chance?  As in, are we sure the SetDocument call won't kill off aDocument?

>Index: content/xul/Makefile.in
>-DIRS		= content document templates
>+DIRS		= document

Add a comment saying why we still need to build content/xul/document

>Index: dom/public/nsIDOMClassInfo.h
>Index: dom/src/base/nsDOMClassInfo.cpp

Changing the order of the classinfo decls breaks backwards compat for some
things that we want backwards compat for.  It's fine if minimo builds are not
compatible with mozilla proper, but please don't change the order in the
MOZ_XUL case.  Yes, it means an extra pair of ifdefs.  That's unfortunate... 

>Index: embedding/components/Makefile.in
>-DIRS = windowwatcher appstartup find ui printingui jsconsole webbrowserpersist commandhandler build 
>+DIRS = windowwatcher appstartup find webbrowserpersist commandhandler ui jsconsole

Add a comment explaining why those dirs get built unconditionally

With those, r=bzbarsky

Comment 28

[Brian Ryner (not reading)]

this was checked in.

Comment 29

[Simon Fraser [no longer active]]

Can we turn XUL off for camino?

Comment 30

[Brendan Eich [:brendan]]

Do you really gain much by turning off XUL for camino?  You lose the ability to
do remote XUL for things like the amazon XUL browser
(http://www.infodraft.com/%7Efaser/mab/), for XUL widgets in-page in the near
term as a better forms (X-forms, web services dialogs) basis, etc.

What's the static footprint gain, and when compressed for download?  What's the
startup gain?

Those are good to know, but they're apples to oranges when trading off the value
of XUL for things like the amazon browser.  I think all of the Mozilla-built or
"tier 1" browsers should support XUL.

/be