340510 - nsIDOMNSDocument.getBoxObjectFor (standard way to use Mozilla) does not work with --disable-xul

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Oleg Romashin (:romaxa)]

Building mozilla form latest trunk sources with option --disable-xul  failed.


Reproducible: Always

Steps to Reproduce:
1. cvs -z3 -d :pserver:[e-mail]:/cvsroot co
mozilla/client.mk
2. cp mozilla/xulrunner/config/mozconfig mozilla/mozconfig
3. Modify mozconfig like here:
...........................................
# This file specifies the build flags for XULRunner.  You can use it by adding:
#  . $topsrcdir/xulrunner/config/mozconfig
# to the top of your mozconfig file.

mk_add_options MOZ_CO_PROJECT=xulrunner
ac_add_options --enable-application=xulrunner
ac_add_options --enable-default-toolkit=gtk2
ac_add_options --disable-pango
ac_add_options --enable-xft
ac_add_options --disable-freetype2
ac_add_options --with-embedding-profile=default
ac_add_options --disable-javaxpcom
ac_add_options --enable-debug
ac_add_options --enable-logging
ac_add_options --enable-optimize=" -O0 -g3"
ac_add_options --disable-xul
ac_add_options --disable-accessibility
ac_add_options --disable-inspector-apis
........................................

3. Apply RDF disable patch 140_BUILDFIX_rdf_disable.dpatch from attachment: needed for building with --disable-xul option:

4. make -f client.mk checkout
autoconf && ./configure && make

Actual Results:  
calendar.google.com works wrong without main task table.
Screenshot and output log in attachment.

Expected Results:  
calendar.google.com works properly.

PS: 

This build works fine with maps.google.com, gmail.com...

Problems only with calendar.google.com.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Screenshot of xulrunner with disabled xul on calendar.google.com

Screenshot of xulrunner with disabled xul on calendar.google.com

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Console log for this bug

Comment 3

[Oleg Romashin (:romaxa)]

Attachment: Patch for disabling RDF service with disabled xul

Comment 4

[Robert Accettura [:raccettura]]

I'm guessing this goes to core-->buildconfig

Comment 5

[timeless]

[e-mail]: sorry, it doesn't.

this goes to dom.

here's an explanation:

1. as we all know from bug 334765, google calendar uses getBoxObjectFor because the web sucks (ref bug 174397 comment 15).
2. since google recognizes that romaxa's browser is a gecko, it tries to use getBoxObjectFor.
3. romaxa disabled xul (this is something we're supposed to be able to do, it's supposed to work -- and romaxa will make it work, although it'd be nice if mw or neil or someone would help, since this is a glass shop and it's not a place i'd like romaxa to try to touch).
4. hyatt made dom depend on xul
5. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsDocument.cpp&rev=3.651&mark=3201-3222,3224,3225-3226#3172

getBoxObjectFor depends on xul, we actually already knew this, but I can't remember where.

I believe that the javascript:document.getBoxObjectFor(document.body) is the simplest testcase. romaxa will confirm shortly.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 224545 [details] [diff] [review]
Patch for disabling RDF service with disabled xul

Could you please attach a cvs diff? You should then ask for review by selecting the "review ?" flag and entering ":bs" in the text field.

Comment 7

[Oleg Romashin (:romaxa)]

./run-mozilla.sh ./TestGtkEmbed "javascript:document.getBoxObjectFor(document.body)"

Shows empty page, and log:
....................................
WARNING: Cannot create startup observer : service,@mozilla.org/updates/update-service;1: file nsAppStartupNotifier.cpp, line 112
GFX: dpi=96 t2p=0.0666667 p2t=15 depth=24
++WEBSHELL 0x828d270 == 1
WARNING: EnableGlobalHistory() failed: 'NS_SUCCEEDED(rv)', file nsWebBrowser.cpp, line 1222
++DOMWINDOW == 1
loading url javascript:document.getBoxObjectFor(document.body)
CSS Error (chrome://global/content/xul.css :202.25): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :328.22): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :333.22): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :378.21): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :383.27): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :388.26): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :455.21): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :463.27): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :467.26): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :570.21): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :575.22): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :644.21): Error in parsing value for property 'display'.  Declaration dropped.
CSS Error (chrome://global/content/xul.css :730.22): Error in parsing value for property 'display'.  Declaration dropped.
js_status_cb
++DOMWINDOW == 2
js_status_cb
location_changed_cb
open_uri_cb javascript:document.getBoxObjectFor(document.body)
JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMNSDocument.getBoxObjectFor]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: javascript:document.getBoxObjectFor(document.body) :: <TOP_LEVEL> :: line 1"  data: no]
...................................................

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: Patch for disabling RDF service with disabled xul Second edition

Comment 9

[Oleg Romashin (:romaxa)]

(In reply to comment #8)
> Created an attachment (id=224559) [edit]
> Patch for disabling RDF service with disabled xul Second edition
> 


For this patch we have separate BUG, I have updated it.
Added MOZ_RDF , and --disable-rdf option

https://bugzilla.mozilla.org/show_bug.cgi?id=330331

Comment 10

[Boris Zbarsky [:bzbarsky]]

So yeah... we should probably build the vanilla box object (the one used for the world) even if XUL is disabled...

Comment 11

[timeless]

Attachment: this is an untested patch

this patch requires to cvs copies to be done after applying the patch.

if the cvs copies are done before applying the patch, then the patch would need to be adapted to the cvs copies. i'm too lazy at this hour to write the patch based on a future cvs copy.

they are:
cp mozilla/layout/xul/base/public/nsIBoxObject.idl,v mozilla/layout/base
cp mozilla/layout/xul/base/src/nsBoxObject.cpp,v mozilla/layout/base

If you aren't using a cvs repository, omit the ',v' as that's the rcs record format.

anyway, i'd imagine romaxa will test this in 8hrs or so and we'll find out what silly errors i made. i have other bug triaging to do. and i'm hoping that roc will just remove this api entirely and solve the problem by saving the web from this api that shouldn't be there.

Comment 12

[Benjamin Smedberg]

> 3. romaxa disabled xul (this is something we're supposed to be able to do, it's
> supposed to work -- and romaxa will make it work, although it'd be nice if mw

It's only supposed to work in conjunction with the minimal profile defined at http://wiki.mozilla.org/Gecko:Small_Device_Support

That said, I think the fix here is roughly the correct solution.

Comment 13

[Oleg Romashin (:romaxa)]

Attachment: this is a tested patch

as before, this patch is to be applied and then the following cvs copies are to be made:
cp mozilla/layout/xul/base/public/nsIBoxObject.idl,v mozilla/layout/base
cp mozilla/layout/xul/base/public/nsPIBoxObject.h,v mozilla/layout/base
cp mozilla/layout/xul/base/src/nsBoxObject.cpp,v mozilla/layout/base
cp mozilla/layout/xul/base/src/nsBoxObject.h,v mozilla/layout/base

realistically bz will probably say that some of these files should actually go elsewhere, and the way this change would actually work, the files would be copied and the deltas would be applied to the new locations, but this patch allows for cvs diffs/patches to work today.

(and of course, there's always the chance that roc kills the feature before this patch gets reviews)

Comment 14

[Boris Zbarsky [:bzbarsky]]

I'm not going to be able to get to this review in the foreseeable future (esp. because I'd need to figure out how this affects the consumers of nsIBoxObject, etc).  Foreseeable future is probably until July at least.

I'd suggest asking someone more familiar with this code for review.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You're changing the interface without revving the IID and like bz, I'm not sure that this change isn't going to break XUL apps.

Why not just bring nsIBoxLayoutManager and nsIBoxPaintManager into #ifndef XUL builds? They don't have any dependencies of their own.

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 224747 [details] [diff] [review]
this is a tested patch

I like the patch in principle, but I also think changing the interface is a bad idea.  nsIBoxLayoutManager and nsIBoxPaintManager should be forward-declared in the idl and continue to be #included in the implementation.

Comment 17

[timeless]

this is ridiculous. bug 341245 removed the interfaces you guys are quibling over. and no one actively protested about changing interfaces. sure roc warned smaug that he might break people, but he didn't fight it.

Comment 18

[timeless]

Comment on attachment 224747 [details] [diff] [review]
this is a tested patch

>Index: dom/public/nsDOMClassInfoID.h
personally. I think we need to stop having ifdefs around the enums in this file, it screws with binary compatibility. It's ok to not have the actual name, so that people trying to compile will know that it won't work, something like:

#ifdef MOZ_XUL
   eDOMClassInfo_TreeSelection_id,
   eDOMClassInfo_TreeContentView_id,
#else
   eDOMClassInfo_PLACEHOLDER_DO_NOT_USE_10,
   eDOMClassInfo_PLACEHOLDER_DO_NOT_USE_11,
#endif

But now that most of the interesting pieces of this patch are gone (and they died 3 weeks ago), this patch is obsolete.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

When I wrote my comments here, I didn't realize that nsIBoxLayoutManager and nsIBoxPaintManager have actually never been implemented ...

Comment 20

[Oleg Romashin (:romaxa)]

Attachment: Same patch, but without copying files...

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 278379 [details] [diff] [review]
Same patch, but without copying files...

Take out the "timeless" comments

Comment 22

[Reed Loden [:reed]]

Attachment: unbitrotten patch with nit addressed

Comment 23

[Reed Loden [:reed]]

Checking in dom/src/base/nsDOMClassInfo.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMClassInfo.cpp,v  <--  nsDOMClassInfo.cpp
new revision: 1.515; previous revision: 1.514
done
Checking in dom/public/nsDOMClassInfoID.h;
/cvsroot/mozilla/dom/public/nsDOMClassInfoID.h,v  <--  nsDOMClassInfoID.h
new revision: 1.33; previous revision: 1.32
done
Checking in layout/xul/base/src/nsBoxObject.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsBoxObject.cpp,v  <--  nsBoxObject.cpp
new revision: 1.72; previous revision: 1.71
done
Checking in layout/base/Makefile.in;
/cvsroot/mozilla/layout/base/Makefile.in,v  <--  Makefile.in
new revision: 1.41; previous revision: 1.40
done
Checking in layout/build/nsLayoutModule.cpp;
/cvsroot/mozilla/layout/build/nsLayoutModule.cpp,v  <--  nsLayoutModule.cpp
new revision: 1.184; previous revision: 1.183
done