Closed Bug 1183733 Opened 9 years ago Closed 6 years ago

Second app update after a compatibility override re-enables an incompatible addon

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED INACTIVE

People

(Reporter: rkent, Unassigned)

References

Details

Attachments

(1 file)

STR:

For purposes of the STR I will give my current example, which uses three versions of Thunderbird, and the FoxyProxy addon which was set as incompatible for Thunderbird > 38.0a2  But this is a core issue, and the same scenario in Firefox should have the same issue.

1) Create a profile running Thunderbird 31.7.0  Install FoxyProxy (which installs correctly).
2) Run that profile using Thunderbird 38.0.1. Expected and actual result: incompatibility is detected, and the addon is disabled. (extension remains disabled with multiple restarts using Thunderbird 38.0.1)
3) Run that profile using Thunderbird 38.1.0

Expected result: FoxyProxy stays disabled, reports as incompatible.
Actual result: FoxyProxy is now enabled and loads. Addon Manager reports addon as incompatible and gives user option to disable it if Addon Manager is opened.

Due to the nature of the FoxyProxy crash (which is unrelated to the issues of this bug), on actual released versions of Thunderbird step 3 actually crashes seconds after startup. But with local builds this does not happen, so I was able to observe the pattern of the re-enabling of the addon.

Analysis:

At step 3), addon status is userdisabled: false, appDisabled: true, softDisabled: false.

Detection of appDisabled is done in the method isCompatibleWith in XPIProvider.jsm. There is code there to check compatibility, executed under this if statement:

      // The repository can specify compatibility overrides.
      // Note: For now, only blacklisting is supported by overrides.
      if (this._repositoryAddon &&
          this._repositoryAddon.compatibilityOverrides) {

The problem is that _repositoryAddon is undefined as that has not yet been initialized. As a result, the compatibility override is not actually checked, the method ultimately returns true, and .appDisabled gets reset to false, and the addon loads.
This patch is really meant more to demonstrate the problem rather than the proposed long-term solution. But it might be viable as the correct handling of the error of a missing _repositoryAddon, and it might be a viable branch fix to get around the current Thunderbird issues.

It seems to me that the correct fix would be to make sure that _repositoryAddon is enabled at this point, but that requires inserting of an async call somewhere in a stack of sync calls. I don't understand the design of the addon manager code well enough to propose the correct way to do that.
Blocks: 1083545
See Also: → 1137274
See Also: → 1183890
Robert, can you comment on comment 1?
Flags: needinfo?(robert.strong.bugs)
See Also: → 1189270
(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #2)
> Robert, can you comment on comment 1?
Wayne, it would be much better to get an add-ons manager owner / peer to look into that since it is add-ons manager code.
Flags: needinfo?(robert.strong.bugs)
Comment on attachment 8633589 [details] [diff] [review]
Fix the immediate symptom

(In reply to Kent James (:rkent) from comment #1)
> Created attachment 8633589 [details] [diff] [review]
> Fix the immediate symptom
> 
> This patch is really meant more to demonstrate the problem rather than the
> proposed long-term solution. But it might be viable as the correct handling
> of the error of a missing _repositoryAddon, and it might be a viable branch
> fix to get around the current Thunderbird issues.
> 
> It seems to me that the correct fix would be to make sure that
> _repositoryAddon is enabled at this point, but that requires inserting of an
> async call somewhere in a stack of sync calls. I don't understand the design
> of the addon manager code well enough to propose the correct way to do that.

You probably need to talk to Mossop
Attachment #8633589 - Flags: feedback?(dtownsend)
Comment on attachment 8633589 [details] [diff] [review]
Fix the immediate symptom

Review of attachment 8633589 [details] [diff] [review]:
-----------------------------------------------------------------

Just persisting appDisabled here is going to have all kinds of bad side effects. Unfortunately I think the right solution is harder, caching in the add-ons database that a compatibility override is in place for the add-on.
Attachment #8633589 - Flags: feedback?(dtownsend) → feedback-
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: