Closed Bug 996074 Opened 6 years ago Closed 6 years ago

Completely remove xpcnativewrappers=no from DOMi chrome.manifest

Categories

(Other Applications :: DOM Inspector, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla32

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

Attachments

(1 file, 5 obsolete files)

The SeaMonkey-bundled DOM Inspector addon still comes with a chrome.manifest containing an "xpcnativewrappers=no" directive, which is greatly outdated and causes a warning to appear in the JS console on startup of SeaMonkey; it should be removed as DOM Inspector does not use XPC native wrappers.
Note that bug 533599 was apparently supposed to do this, but doesn't seem to have completely removed all 'xpcnativewrappers' directives.
Here's my proposed patch to fix the bug.
Whiteboard: push patch into dom-inspector repo
Comment on attachment 8406272 [details] [diff] [review]
Patch to remove the obsolete directive from chrome.manifest

plz review
Attachment #8406272 - Flags: review?(neil)
You should also edit install.rdf and set the minVersion to whenever xpcnativewrappers=no was obsoleted.
Attached patch Patch to remove, version 2 (obsolete) — Splinter Review
OK on the basis that it was removed in Gecko 2.0, I set the minversion to SM 2.1.
Attachment #8406272 - Attachment is obsolete: true
Attachment #8406272 - Flags: review?(neil)
I screwed up the last patch... here's version 3, which is fixed.

Again, on the basis that it was removed in Gecko 2.0, I set the minversion to SM 2.1.
Attachment #8406359 - Attachment is obsolete: true
Attachment #8406392 - Flags: review?(neil)
You need to change all of the minimum versions, including the big scary comment at the beginning.

Since you effectively require that XPCNativeWrapper.unwrap exists, you should also remove the backward compatibility check part of bug 533599.
Attached patch ver4.patch (obsolete) — Splinter Review
Updated patch as per neil's suggestions.
Attachment #8406392 - Attachment is obsolete: true
Attachment #8406392 - Flags: review?(neil)
Attachment #8407211 - Flags: review?(neil)
Comment on attachment 8407211 [details] [diff] [review]
ver4.patch

>     <!-- For maxVersion of applications that follow the rapid cycle pattern,
>          use '(x)a1' (*-central) on default,
>          and '(x-1).*' (*-aurora/beta/release) for branches. -->
AMO doesn't like us uploading new DOM Inspector builds with unreleased version numbers.

>     <!-- Fennec -->
[Hmm, we should rip out Fennec support (and its overlay) (in a separate bug).]

>+        <em:maxVersion>32.0</em:maxVersion>
So the maximum this can be is 31a1 (right now) or 32a1 (after uplift) but not 32.0 (similarly for the other versions).

>+        <em:maxVersion>2.27.*</em:maxVersion>
And strangely this one is too low, not that we've been keeping the versions in sync ;-)

>     <!-- Sunbird -->
[And Sunbird (and its overlay) should go too.]

>+        <em:maxVersion>32.0</em:maxVersion>
Ditto.

>+        <em:maxVersion>32.0</em:maxVersion>
Ditto.
(In reply to neil@parkwaycc.co.uk from comment #9)
> Comment on attachment 8407211 [details] [diff] [review]
> ver4.patch
> 
> >     <!-- For maxVersion of applications that follow the rapid cycle pattern,
> >          use '(x)a1' (*-central) on default,
> >          and '(x-1).*' (*-aurora/beta/release) for branches. -->
> AMO doesn't like us uploading new DOM Inspector builds with unreleased
> version numbers.

Curious, I looked at the maximum version I could bump my extension's compatibility to on AMO to get the maxversion.  Anyway, with the Firefox rapid release cycle, that problem will be sorted in a week.  Two at most.  :-)
(In reply to Jeremy Morton from comment #10)
> (In reply to from comment #9)
> > (From update of attachment 8407211 [details] [diff] [review])
> > >     <!-- For maxVersion of applications that follow the rapid cycle pattern,
> > >          use '(x)a1' (*-central) on default,
> > >          and '(x-1).*' (*-aurora/beta/release) for branches. -->
> > AMO doesn't like us uploading new DOM Inspector builds with unreleased
> > version numbers.
> 
> Curious, I looked at the maximum version I could bump my extension's
> compatibility to on AMO to get the maxversion.

Indeed, I see they offer 32.0 for Firefox and Thunderbird. I admit I was just reading from the bug that added that comment. (SeaMonkey version is still wrong though?) But I'd still like to stick with the a1 values for consistency.
Comment on attachment 8407211 [details] [diff] [review]
ver4.patch

r=me if you switch to the current/next nightly version numbers.
Attachment #8407211 - Flags: review?(neil) → review+
Attached patch patch v5 (obsolete) — Splinter Review
New version of patch.
Attachment #8407211 - Attachment is obsolete: true
Attachment #8411746 - Flags: review?(neil)
Attached patch patch v6Splinter Review
Patch v6.
Attachment #8411746 - Attachment is obsolete: true
Attachment #8411746 - Flags: review?(neil)
Attachment #8411747 - Flags: review?(neil)
Attachment #8411747 - Flags: review?(neil) → review+
(What to do about Fennec and Sunbird?)
(In reply to neil@parkwaycc.co.uk from comment #15)
> (What to do about Fennec and Sunbird?)
Sunbird is DOA. Sunbird specific code has been removed from /calendar/

Pushed to DOM Inspector repository
http://hg.mozilla.org/dom-inspector/rev/42d670ba3bb3

[push patch into dom-inspector repo]
Jeremy, in future please use the checkin-needed keyword.
Status: NEW → RESOLVED
Closed: 6 years ago
Component: General → DOM Inspector
Product: SeaMonkey → Other Applications
Resolution: --- → FIXED
Whiteboard: push patch into dom-inspector repo
Target Milestone: --- → mozilla32
Assignee: nobody → bugzilla
Blocks: DOMi2.0.15
You need to log in before you can comment on or make changes to this bug.