576910 - Make DOM-inspector XPCOM components use new manifests and data tables

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Justin Wood (:Callek)]

+++ This bug was initially created as a clone of Bug #576869 +++

Bug 568691 has changed XPCOM registration a lot, DOMi is now broken and when trying to compile you get:
mozilla/config/rules.mk:1822: *** .js component without matching .manifest.  Stop.
In the extensions/inspector directory.

Comment 1

[Justin Wood (:Callek)]

Hey Fallen, I heard you already tackled this but recieved a r-; I'm not sure where your work is though.

Comment 2

[Justin Wood (:Callek)]

Attachment: v1.0

Comment 3

[Ian Neal]

Comment on attachment 455990 [details] [diff] [review]
v1.0

As we only need to support 1.9.0 and upwards, you don't need to keep around the nsICmdLineHandler or the non-XPCOMUtils stuff.

Comment 4

[Ian Neal]

Attachment: Drop support for xpfe versions patch v1.1

This is a version of the patch if it is decided to drop support for non-toolkit (xpfe) consumers of DOMi (i.e. only works with apps that use Gecko 1.9.0+ so not SM1.x).

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 455998 [details] [diff] [review]
Drop support for xpfe versions patch v1.1

If we take this we need more...

#   // properties required for XPCOM registration:  
   classDescription: "unique text description",  
   classID:          Components.ID("{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}"),  
   contractID:       "@example.com/xxx;1",  

And a proper use of: |_xpcom_categories| for XPCOMUtils use in <Mozilla2.

Comment 6

[Ian Neal]

Attachment: Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]

Changes since v1.1:
* As per feedback.

Comment 7

[Philip Chee]

> +  contractID: "@mozilla.org/commandlinehandler/general-startup;1?type=inspector",
Note this format is only needed for XPFE. Toolkit doesn't care about the format (in general) except that they are unique. Suggest you use something shorter like:
@mozilla.org//inspector-clh;1

Comment 8

[Philip Chee]

> @mozilla.org//inspector-clh;1
Err, I meant @mozilla.org/domi/inspector-clh;1

Comment 9

[Colby Russell :crussell (no longer Mozilla-ing)]

One (minor) advantage the first form has though, is that if you are looking at a list of classes, all the command-line handlers are grouped together (assuming the contract IDs are sorted, and everybody else is doing the same).

Comment 10

[Philip Chee]

However I think almost everyone else has moved to the toolkit style contracts for command line handlers:
http://mxr.mozilla.org/mozilla-central/search?string=command-line-handler&find=%5C.manifest

Comment 11

[Justin Wood (:Callek)]

(In reply to comment #10)
> However I think almost everyone else has moved to the toolkit style contracts
> for command line handlers:
> http://mxr.mozilla.org/mozilla-central/search?string=command-line-handler&find=%5C.manifest

Thats fine, but at this stage I don't think we should change the contract

Comment 12

[Justin Wood (:Callek)]

Comment on attachment 456092 [details] [diff] [review]
Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]

Looks good. Shawn if you are willing to just rs this, or consider me reviewer-worthy for this, we can take my feedback as a r+

Comment 14

[Shawn Wilsher :sdwilsh]

Comment on attachment 456092 [details] [diff] [review]
Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]

rs=sdwilsh

Comment 15

[Ian Neal]

Comment on attachment 456092 [details] [diff] [review]
Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]

http://hg.mozilla.org/dom-inspector/rev/1ee8f0bdccfb

Comment 16

[[e-mail]]

Comment on attachment 456092 [details] [diff] [review]
Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]

>+#ifdef XPI_NAME
I thought XPI_NAME was always set.

Comment 17

[Colby Russell :crussell (no longer Mozilla-ing)]

(In reply to comment #16)
> >+#ifdef XPI_NAME
> I thought XPI_NAME was always set.

Apparently not.  My hacky build scripts (based on KaiRo's scripts for building a standalone ChatZilla) preprocessed those lines out.  See bug 590537.