Closed Bug 576910 Opened 11 years ago Closed 10 years ago

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

Categories

(Other Applications :: DOM Inspector, defect)

defect
Not set
normal

Tracking

(blocking-seamonkey2.1 a3+)

RESOLVED FIXED
mozilla2.0
Tracking Status
blocking-seamonkey2.1 --- a3+

People

(Reporter: Callek, Assigned: Callek)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

+++ 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.
No longer depends on: 576869
Summary: Make venkman XPCOM components use new manifests and data tables → Make DOM-inspector XPCOM components use new manifests and data tables
Status: NEW → ASSIGNED
Hey Fallen, I heard you already tackled this but recieved a r-; I'm not sure where your work is though.
Attached patch v1.0Splinter Review
Attachment #455990 - Flags: review?
Attachment #455990 - Flags: feedback?
Attachment #455990 - Attachment is patch: true
Attachment #455990 - Attachment mime type: application/octet-stream → text/plain
Attachment #455990 - Flags: review?(neil)
Attachment #455990 - Flags: review?
Attachment #455990 - Flags: feedback?(iann_bugzilla)
Attachment #455990 - Flags: feedback?
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.
Attachment #455990 - Flags: feedback?(iann_bugzilla) → feedback-
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 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.
Attachment #455998 - Flags: feedback-
Changes since v1.1:
* As per feedback.
Attachment #455998 - Attachment is obsolete: true
Attachment #456092 - Flags: feedback?(bugspam.Callek)
> +  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
> @mozilla.org//inspector-clh;1
Err, I meant @mozilla.org/domi/inspector-clh;1
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).
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
(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 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+
Attachment #456092 - Flags: review?(sdwilsh)
Attachment #456092 - Flags: feedback?(bugspam.Callek)
Attachment #456092 - Flags: feedback+
Attachment #455990 - Flags: review?(neil)
Duplicate of this bug: 576791
Comment on attachment 456092 [details] [diff] [review]
Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]

rs=sdwilsh
Attachment #456092 - Flags: review?(sdwilsh)
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
Attachment #456092 - Attachment description: Drop support for xpfe versions patch v1.2 → Drop support for xpfe versions patch v1.2 [Checkin: Comment 15]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
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.
(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.
You need to log in before you can comment on or make changes to this bug.