Closed Bug 1094694 Opened 5 years ago Closed 5 years ago

SeaMonkey changes needed due to the web installer interfaces now using browsers instead of DOM windows

Categories

(SeaMonkey :: General, defect)

defect
Not set

Tracking

(seamonkey2.30 wontfix, seamonkey2.31? fixed, seamonkey2.32? fixed, seamonkey2.33 fixed)

RESOLVED FIXED
seamonkey2.33
Tracking Status
seamonkey2.30 --- wontfix
seamonkey2.31 ? fixed
seamonkey2.32 ? fixed
seamonkey2.33 --- fixed

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(2 files, 1 obsolete file)

(From Bug 1084558 comment #1)
> Well there is a quick fix here but the problem stems from the add-on manager
> using a number of APIs that in non-e10s mode take DOM windows and in e10s
> mode take browser elements. This causes some confusion so I decided to unify
> to use browser elements everywhere.
Sorry I meant to file these bugs but forgot when bug 1084558 actually got to the point of landing. I remember looking over the comm-central code and thinking it would be pretty straightforward to support the changed but please let me know if you hit any issues.
Attached patch Patch v1.0 Proposed fix. (obsolete) — Splinter Review
> -                  installs.forEach(function(aInstall) {
> +                  installInfo.installs.forEach(function(aInstall) {
Fixed a typo. Too much copy-pasta from Firefox?
Attachment #8518955 - Flags: review?(neil)
(In reply to Philip Chee from comment #2)
> (From update of attachment 8518955 [details] [diff] [review])
> > -                  installs.forEach(function(aInstall) {
> > +                  installInfo.installs.forEach(function(aInstall) {
> Fixed a typo. Too much copy-pasta from Firefox?
Oops. Can we uplift this fix?
Comment on attachment 8518955 [details] [diff] [review]
Patch v1.0 Proposed fix.

>+            var installInfo = aSubject instanceof Components.interfaces.amIWebInstallInfo ? aSubject : null;
>+            var isActiveBrowser = installInfo && installInfo.browser == browser;
...
>               case "addon-install-blocked":
>-                var installInfo = aSubject.QueryInterface(Components.interfaces.amIWebInstallInfo);
>-                if (installInfo.originator.top == browser.contentWindow)
>+                if (isActiveBrowser)
Sorry, I don't like this change. Please can you switch it back (except the actual contentWindow change of course)?
> Sorry, I don't like this change. Please can you switch it back (except 
> the actual contentWindow change of course)?
Switched back.
Attachment #8518955 - Attachment is obsolete: true
Attachment #8518955 - Flags: review?(neil)
Attachment #8519899 - Flags: review?(neil)
Attachment #8519899 - Flags: review?(neil) → review+
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/0ee12c1317ec
comm-central changeset 0ee12c1317ec
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.33
>> Fixed a typo. Too much copy-pasta from Firefox?
> Oops. Can we uplift this fix?
Attachment #8523426 - Flags: review?(neil)
Attachment #8523426 - Flags: review?(neil) → review+
Comment on attachment 8523426 [details] [diff] [review]
Patch v3.0 Typo fix for comm-aurora.

[Approval Request Comment]
Regression caused by (bug #): Bug 595810 [typo fix]
User impact if declined: Restarting a cancelled addon download probably doesn't work
Testing completed (on m-c, etc.): Baked for a while on comm-central.
Risk to taking this patch (and alternatives if risky): no risk typo fix.
String changes made by this patch: none
Attachment #8523426 - Flags: approval-comm-beta?
Attachment #8523426 - Flags: approval-comm-aurora?
(In reply to Philip Chee from comment #8)
> Comment on attachment 8523426 [details] [diff] [review]
> Patch v3.0 Typo fix for comm-aurora.
> 
> [Approval Request Comment]
> Regression caused by (bug #): Bug 595810 [typo fix]
> User impact if declined: Restarting a cancelled addon download probably
> doesn't work
> Testing completed (on m-c, etc.): Baked for a while on comm-central.
> Risk to taking this patch (and alternatives if risky): no risk typo fix.
> String changes made by this patch: none
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(iann_bugzilla)
Attachment #8523426 - Flags: approval-comm-beta?
Attachment #8523426 - Flags: approval-comm-beta+
Attachment #8523426 - Flags: approval-comm-aurora?
Attachment #8523426 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.