Closed Bug 576791 Opened 14 years ago Closed 14 years ago

Adapt DOM Inspector for mozilla2 xpcom changes

Categories

(Other Applications :: DOM Inspector, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 576910

People

(Reporter: Fallen, Assigned: Fallen)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
After bug 568691 landed, dom inspector command line handler registration isn't correct anymore. This patch fixes. Unfortunately, it seems we need a both the manifest file and a jar.mn. I filed bug 576767, lets hope its reopened.

From what I could see, Seamonkey now uses the toolkit commandline handler, so there's no need for specialcasing.

Also, who should I pick for superreview here?
Attachment #455888 - Flags: review?(neil)
Attached patch Fix - v2 β€” β€” Splinter Review
Forgot to bump the install.rdf version
Attachment #455888 - Attachment is obsolete: true
Attachment #455889 - Flags: review?(neil)
Attachment #455888 - Flags: review?(neil)
Comment on attachment 455889 [details] [diff] [review]
Fix - v2

>-EXTRA_COMPONENTS=inspector-cmdline.js
>+EXTRA_COMPONENTS = inspector-cmdline.js inspector-cmdline.manifest
Instead of this, why not set NO_JS_MANIFEST to 1?

>   handle : function handler_handle(cmdLine) {
>-    var args = Components.classes["@mozilla.org/supports-string;1"]
>-                         .createInstance(nsISupportsString);
>+    let args = Components.classes["@mozilla.org/supports-string;1"]
>+                         .createInstance(Components.interfaces.nsISupportsString);
>     try {
>-      var uristr = cmdLine.handleFlagWithParam("inspector", false);
>-      if (uristr == null)
>+      let uristr = cmdLine.handleFlagWithParam("inspector", false);
>+      if (uristr == null) {
>         return;
>+      }
>       try {
>         args.data = cmdLine.resolveURI(uristr).spec;
>-      }
>-      catch (e) {
>+      } catch (e) {
>         return;
>       }
>-    }
>-    catch (e) {
>+    } catch (e) {
The rewriting was unnecessary and detracts from the patch.

>+    Services.ww.openWindow(null, "chrome://inspector/content/", "_blank",
>+                           "chrome,dialog=no,all", args);
You can't do this, DOM Inspector needs to install in Firefox 3.0

>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([InspectorCmdLineHandler]);
And you also need to do something about this. At least XPCOMUtils existed in Firefox 3.0 so you can easily generate the module for older versions.

>diff --git a/base/js/jar.mn b/base/js/jar.mn
Why not add the lines to the existing jar.mn?

>+        <em:maxVersion>4.0b2pre</em:maxVersion>
This is only valid if AMO has already updated (I didn't check though).
Attachment #455889 - Flags: review?(neil) → review-
(In reply to comment #2)
>(From update of attachment 455889 [details] [diff] [review])
>>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([InspectorCmdLineHandler]);
>And you also need to do something about this. At least XPCOMUtils existed in
>Firefox 3.0 so you can easily generate the module for older versions.
Although you'll need to add the 3.0 command line category bits.
(In reply to comment #0)
> Also, who should I pick for superreview here?
you do not need it
(In reply to comment #2)
> You can't do this, DOM Inspector needs to install in Firefox 3.0
I'm OK with dropping support for Firefox 3.0.  It's been end of lifed (but we shouldn't drop support if it also breaks a supported version of Seamonkey).
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: