Closed Bug 453545 Opened 13 years ago Closed 13 years ago

InstallTrigger Broken when doing multiple signed XPI installs

Categories

(Core Graveyard :: Installer: XPInstall Engine, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: pete, Assigned: mossop)

References

()

Details

(Keywords: verified1.9.0.5)

Attachments

(2 files)

Steps to reproduce:

  1. go to this page: https://www.mozdevgroup.com/clients/bm/
  2. click on the "Combined Install" link
  3. two signed XPI's are installed
  4. second XPI fails w/ a "Signing could not be verified" error.

Expected result:
  4. both XPI's are successfully installed

This is only an issue w/ FF3. The above works correctly w/ FF2.
Attachment #336733 - Attachment description: screen-shop of alert error → screen-shot of alert error
You can install the two XPI's individually though ...
Component: Installer → Installer: XPInstall Engine
Product: Firefox → Core
QA Contact: installer → xpi-engine
Version: unspecified → 1.9.0 Branch
Summary: InstallTrigger Broken when doing multiple XPI installs → InstallTrigger Broken when doing multiple signed XPI installs
Assignee: nobody → dtownsend
Version: 1.9.0 Branch → Trunk
Blocks: 462108
Flags: wanted1.9.0.x+
Attached patch patch rev 1Splinter Review
This is a pretty simple problem. The manifest hash isn't emptied when the zipreader is closed. Since xpinstall uses a single instance of zipreader to parse all installs the second one fails because it appears that an entry is duplicated in the manifest.
Attachment #347871 - Flags: review?
Attachment #347871 - Flags: review? → review?(cbiesinger)
Status: NEW → ASSIGNED
Attachment #347871 - Flags: review?(cbiesinger) → review+
Comment on attachment 347871 [details] [diff] [review]
patch rev 1

I'd like to get this landed a.s.a.p. as this issue is causing problems for AMO and I would like to get a branch fix done.
Attachment #347871 - Flags: approval1.9.1b2?
Target Milestone: --- → mozilla1.9.1b2
Attachment #347871 - Flags: approval1.9.1b2? → approval1.9.1b2+
Pushed: http://hg.mozilla.org/mozilla-central/rev/4abab3456109
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Hot!
Comment on attachment 347871 [details] [diff] [review]
patch rev 1

Looking for approval to land this on the branch before code freeze. This is a trivial fix that makes the zip reader properly reusable when testing signed zips. This has been on trunk for a few days with no reported issues.
Attachment #347871 - Flags: approval1.9.0.5?
(In reply to comment #7)
> This has been on trunk for a few days with no reported issues.

Agreed: I just tried installing two signed xpis together with Minefield, and unlike on Fx 3.0.4, it works great.
Flags: in-litmus?
Comment on attachment 347871 [details] [diff] [review]
patch rev 1

Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #347871 - Flags: approval1.9.0.5? → approval1.9.0.5+
Checked in to the 1.9 tree:

Checking in modules/libjar/nsJAR.cpp;
/cvsroot/mozilla/modules/libjar/nsJAR.cpp,v  <--  nsJAR.cpp
new revision: 1.132; previous revision: 1.131
done
Keywords: fixed1.9.0.5
Verified that this now works with 1.9.0.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008120105 GranParadiso/3.0.5pre.
Flags: in-testsuite? → in-testsuite+
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 ID:20090624012136

Dave, do we really still need a Litmus test or is it already covered by the automated test?
Status: RESOLVED → VERIFIED
(In reply to comment #12)
> Dave, do we really still need a Litmus test or is it already covered by the
> automated test?

We have no automated test for the 1.9.0.x branch so a litmus test may be helpful there.
(In reply to comment #13)
> (In reply to comment #12)
> > Dave, do we really still need a Litmus test or is it already covered by the
> > automated test?
> 
> We have no automated test for the 1.9.0.x branch so a litmus test may be
> helpful there.

But we have for 1.9.1? Have I understood your last comment correctly?
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > Dave, do we really still need a Litmus test or is it already covered by the
> > > automated test?
> > 
> > We have no automated test for the 1.9.0.x branch so a litmus test may be
> > helpful there.
> 
> But we have for 1.9.1? Have I understood your last comment correctly?

That is correct. To land the automated changes on 1.9.0 would have required backporting a significant number of changes to the test harnesses.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.