293583 - When phone home occurs for more than one extension the update info is used from the first extension update info

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Robert Strong (they/them - no direct email)]

It appears that when installing more than one incompatible extension the update
info (e.g. minVersion / maxVersion) from the first extension checked is used for
all of the extensions installed.

I found this by accident when I had an incorrect update rdf for the second
extension I was installing during a tewo extension install. The first extension
was also incompatible but had a correct update rdf. They both installed
successfully. I then placed a couple of dump statements in the
IncompatibleObserver::observe within nsExtensionManager.js and found this to be
the case.

Comment 1

[Robert Strong (they/them - no direct email)]

output from the console:
*** Version Check Phone Home Completed
this._xpi.path=<path>\staged-xpis\{34274bf4-1d97-a289-e984-17e546307e4f}\adblock-0.5.2.039-fx.xpi
item.id={5DB6B72E-5C55-47a0-AB3B-945B4023E398}
item.maxAppVersion=1.1

*** Version Check Phone Home Completed
this._xpi.path=<path>\staged-xpis\{5DB6B72E-5C55-47a0-AB3B-945B4023E398}\testcase1.xpi
item.id={5DB6B72E-5C55-47a0-AB3B-945B4023E398}
item.maxAppVersion=1.1

Comment 2

[Robert Strong (they/them - no direct email)]

Attachment: patch

This bug occurs due to that installs and upgrades can start multiple phone
homes since each item is checked individually hence why this doesn't occur
during other updates. This patch simply adds the first item checked to the
Update:Extension:Started and Update:Extension:Started notifyObservers as is
already used with Update:Extension:Item-Ended and Update:Extension:Item-Error.
It is then possible to do an early return in observes when we receive
notification for an item that is different from the item we are actually
checking.

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: patch

Bah... forgot to include update.js in the diff.

Comment 4

[Benjamin Smedberg]

Comment on attachment 184176 [details] [diff] [review]
patch

Depending on where 1.1a1 is, this is a very safe patch to take. But it's not
the end of the world if it doesn't make it.

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch for checkin

fixes an equality check (e.g. s/=/==/) found by gavin.

Comment 6

[Robert Strong (they/them - no direct email)]

patch checked in - thanks db48x - Resolved > Fixed