All users were logged out of Bugzilla on October 13th, 2018

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

RESOLVED FIXED in mozilla1.9beta3

Status

()

--
major
RESOLVED FIXED
13 years ago
6 years ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

Trunk
mozilla1.9beta3
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 5 obsolete attachments)

(Assignee)

Description

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


Reproducible: Always

Steps to Reproduce:
1. cvs -z3 -d :pserver:anonymous@cvs-mirror.mozilla.org:/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.
(Assignee)

Comment 1

13 years ago
Created attachment 224542 [details]
Screenshot of xulrunner with disabled xul on calendar.google.com

Screenshot of xulrunner with disabled xul on calendar.google.com
(Assignee)

Comment 2

13 years ago
Created attachment 224543 [details]
Console log for this bug
(Assignee)

Comment 3

13 years ago
Created attachment 224545 [details] [diff] [review]
Patch for disabling RDF service with disabled xul
Component: Reporter → Build Config
Product: Other Applications → Core
I'm guessing this goes to core-->buildconfig
Assignee: robert → nobody
QA Contact: xul-client → build-config

Comment 5

13 years ago
robert@accettura.com: 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.
Assignee: nobody → general
Component: Build Config → DOM: Mozilla Extensions
QA Contact: build-config → ian
Summary: Mozilla does not work's with calendar.google.com properly with --disable-xul config option → nsIDOMNSDocument.getBoxObjectFor (standard way to use Mozilla) does not work with --disable-xul
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.
Attachment #224545 - Attachment is patch: true
Attachment #224545 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Comment 7

13 years ago
./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]
...................................................
(Assignee)

Comment 8

13 years ago
Created attachment 224559 [details] [diff] [review]
Patch for disabling RDF service with disabled xul Second edition
Attachment #224545 - Attachment is obsolete: true
Attachment #224559 - Flags: review?(benjamin)
(Assignee)

Comment 9

13 years ago
(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
So yeah... we should probably build the vanilla box object (the one used for the world) even if XUL is disabled...
Flags: blocking1.9a2?

Comment 11

13 years ago
Created attachment 224609 [details] [diff] [review]
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

13 years ago
> 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.
(Assignee)

Comment 13

13 years ago
Created attachment 224747 [details] [diff] [review]
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)
Attachment #224559 - Attachment is obsolete: true
Attachment #224609 - Attachment is obsolete: true
Attachment #224747 - Flags: superreview?(bzbarsky)
Attachment #224747 - Flags: review?(bzbarsky)
Attachment #224559 - Flags: review?(benjamin)
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.

Updated

13 years ago
Attachment #224747 - Flags: superreview?(bzbarsky)
Attachment #224747 - Flags: superreview?(bryner)
Attachment #224747 - Flags: review?(roc)
Attachment #224747 - Flags: review?(bzbarsky)
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 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.
Attachment #224747 - Flags: superreview?(bryner) → superreview-

Comment 17

12 years ago
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.
Depends on: 341245

Comment 18

12 years ago
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.
Attachment #224747 - Attachment is obsolete: true
Attachment #224747 - Flags: review?(roc)
When I wrote my comments here, I didn't realize that nsIBoxLayoutManager and nsIBoxPaintManager have actually never been implemented ...
Flags: blocking1.9a2? → blocking1.9-
(Assignee)

Comment 20

11 years ago
Created attachment 278379 [details] [diff] [review]
Same patch, but without copying files...
Assignee: general → nobody
QA Contact: ian → general
Attachment #278379 - Flags: superreview?(roc)
Attachment #278379 - Flags: review?(roc)
Comment on attachment 278379 [details] [diff] [review]
Same patch, but without copying files...

Take out the "timeless" comments
Attachment #278379 - Flags: superreview?(roc)
Attachment #278379 - Flags: superreview+
Attachment #278379 - Flags: review?(roc)
Attachment #278379 - Flags: review+
Assignee: nobody → romaxa
Attachment #278379 - Flags: approval1.9?

Updated

11 years ago
Attachment #278379 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Created attachment 297287 [details] [diff] [review]
unbitrotten patch with nit addressed
Attachment #278379 - Attachment is obsolete: true
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
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
Component: DOM: Mozilla Extensions → DOM
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.