Closed Bug 462739 Opened 13 years ago Closed 13 years ago

Send cookies as appropriate with xpinstall requests

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.1b3

People

(Reporter: mossop, Assigned: mossop)

References

Details

(Keywords: fixed1.9.0.9, fixed1.9.1, Whiteboard: (Fixes AMO sandbox w/ 3rd-party cookies turned off))

Attachments

(1 file, 2 obsolete files)

Attached patch patch rev 1 (obsolete) — Splinter Review
Spun off from bug 437174, I don't think the right fix for xpinstall is just to blanket always or never send cookies. We should be sending cookies as appropriate according to the page initiating the request (the page that called InstallTrigger.install). This patch does that by making our requests in the same load group as the original page. This allows the cookie service to correctly compare the host we are requesting from to the host of the original page and so send or not send cookies.
Attachment #345937 - Flags: review?(dveditz)
Comment on attachment 345937 [details] [diff] [review]
patch rev 1

r/sr=dveditz
Attachment #345937 - Flags: superreview+
Attachment #345937 - Flags: review?(dveditz)
Attachment #345937 - Flags: review+
Comment on attachment 345937 [details] [diff] [review]
patch rev 1

I would like to land this for 1.9.1 to allow people to install AMO sandboxed add-ons when they have third party cookies off.
Attachment #345937 - Flags: approval1.9.1?
Comment on attachment 345937 [details] [diff] [review]
patch rev 1

a191=beltzner
Attachment #345937 - Flags: approval1.9.1? → approval1.9.1+
Attached patch patch for checkin (obsolete) — Splinter Review
Attachment #345937 - Attachment is obsolete: true
Attachment #350489 - Flags: superreview+
Attachment #350489 - Flags: review+
Attachment #350489 - Flags: approval1.9.1+
Attachment #350489 - Attachment is obsolete: true
Attachment #350490 - Flags: superreview+
Attachment #350490 - Flags: review+
Attachment #350490 - Flags: approval1.9.1+
http://hg.mozilla.org/mozilla-central/rev/c99b74e60a17
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Depends on: 473060
This seems appropriate and useful to take in a 3.0.x release
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.7?
Whiteboard: (Fixes AMO sandbox w/ 3rd-party cookies turned off)
Mossop: Sam says I can't block on this, but I would like to fix this in 1.9.0. Will the patches here apply to 1.9.0 or could you come up with a 1.9.0 version?
Flags: blocking1.9.0.7?
Attachment #350490 - Flags: approval1.9.0.7?
(In reply to comment #8)
> Mossop: Sam says I can't block on this, but I would like to fix this in 1.9.0.
> Will the patches here apply to 1.9.0 or could you come up with a 1.9.0 version?

We should not take this on 1.9.0.x until bug 473060 has been resolved. It is currently one of my top priorities but will still be early next week before I have a definite fix.
Attachment #350490 - Flags: approval1.9.0.7?
Comment on attachment 350490 [details] [diff] [review]
patch for checkin

next time, then.
Flags: in-testsuite+
Comment on attachment 350490 [details] [diff] [review]
patch for checkin

We can take this on branch now so long as we also take bug 473060
Attachment #350490 - Flags: approval1.9.0.8?
Where is the test that "in-testsuite+" refers to?
(In reply to comment #12)
> Where is the test that "in-testsuite+" refers to?

It was landed as part of bug 474763
Comment on attachment 350490 [details] [diff] [review]
patch for checkin

Approved for 1.9.0.8, a=dveditz for release-drivers
Attachment #350490 - Flags: approval1.9.0.8? → approval1.9.0.8+
Landed on 1.9 branch:

Checking in xpinstall/src/nsXPInstallManager.cpp;
/cvsroot/mozilla/xpinstall/src/nsXPInstallManager.cpp,v  <--  nsXPInstallManager.cpp
new revision: 1.166; previous revision: 1.165
done
Checking in xpinstall/src/nsXPInstallManager.h;
/cvsroot/mozilla/xpinstall/src/nsXPInstallManager.h,v  <--  nsXPInstallManager.h
new revision: 1.48; previous revision: 1.47
done
Keywords: fixed1.9.0.8
The tests for this don't seem to exist in the 1.9.0 branch (from bug 474763). Is there a way for QA to verify this for 1.9.0.8?
Duplicate of this bug: 430538
Keywords: fixed1.9.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.