Closed Bug 1284407 Opened 4 years ago Closed 4 years ago

IDs are only generated for temporary add-ons loaded from directories, not XPI files

Categories

(Toolkit :: Add-ons Manager, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla51
Tracking Status
firefox48 --- wontfix
firefox49 --- wontfix
firefox50 --- affected
firefox51 --- verified

People

(Reporter: cbadescu, Assigned: aswan)

References

Details

(Whiteboard: triaged)

Attachments

(2 files)

Steps to reproduce:

1.Go to about:debugging page.
2.Click on “Load Temporary Add-on” button.
3.Choose a WebExtension with no id and click on “Open” button.
4.Click again on “Load Temporary Add-on” button and choose another WebExtension with no id.


Expected results:
The WebExtensions with no id are loaded correctly.

Actual results:
The WebExtensions with no id override themselves.

Note/Issues:
Verified on FF50.0a1 (2016-07-04) (Win 7 64-bit).
I have tested this scenario using using .xpi files.
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(kmaglione+bmo)
Summary: WebExtensions with no id override themselves in Temporary Add-ons page → IDs are only generated for temporary add-ons loaded from directories, not XPI files
Duplicate of this bug: 1284439
Here's an example extension to reproduce it. When loading as a temporary add-on, I see this error in the log:

Error: aAddon must include an id, version, and type (resource://gre/modules/addons/XPIProvider.jsm:4724:20) JS Stack trace: this.XPIProvider.callBootstrapMethod@XPIProvider.jsm:4724:20 < this.XPIProvider.installTemporaryAddon<@XPIProvider.jsm:3983:5

When loading a new XPI without an ID, I see this in the log:

WARN	Addon with ID undefined already installed, older version will be disabled
Component: WebExtensions → Add-ons Manager
Blocks: 1283656
Priority: -- → P2
Whiteboard: triaged
Duplicate of this bug: 1273239
Duplicate of this bug: 1293016
Carrying a comment over from bug 1293016, the fact that this code just returns silently instead of throwing also makes it look as if the install succeeded.  It should be changed to throw:
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#4813-4814
Assignee: nobody → aswan
I was able to reproduce this issue on Firefox 51.0a1 (2016-08-25), Firefox 50.0a2 (2016-08-25), Firefox 49 beta 6 and Firefox 48.0.2 under Windows 10 64-bit and Ubuntu 16.04 32-bit. Marking the tracking flags accordingly.
OS: Unspecified → All
Hardware: Unspecified → All
Duplicate of this bug: 1298060
Comment on attachment 8790492 [details]
Bug 1284407 Generate addon id from the path for temporarily loaded xpis

https://reviewboard.mozilla.org/r/78274/#review76996
Attachment #8790492 - Flags: review?(rhelmer) → review+
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/58846175232c
Generate addon id from the path for temporarily loaded xpis r=rhelmer
https://hg.mozilla.org/mozilla-central/rev/58846175232c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Do we want to uplift that in 50? Thanks
I am a bit confused about the expected result because loading a second XPI without id will keep replacing the first XPI without id (this not reproduces for temporary add-ons installed from directories ). Tested on Firefox 51.0a1(2016-09-14) under Windows 7 64-bit.

The only difference is that the following error is displayed in browser console:
1473926347748	addons.xpi	ERROR	Error: aAddon must include an id, version, and type (resource://gre/modules/addons/XPIProvider.jsm:4822:20) JS Stack trace: this.XPIProvider.callBootstrapMethod@XPIProvider.jsm:4822:20 < this.XPIProvider.installTemporaryAddon<@XPIProvider.jsm:4048:11
 
Is this error the only thing that fixes this bug? If yes, considering that Bug 1284439 still reproduces, I think it is not a duplicate of this one and should be reopened.
Flags: needinfo?(aswan)
I'm not sure I understand.  If you temporarily load an XPI file and then temporarily load a new XPI from the exact same path, it should replace the original one, otherwise it should load the new extension and leave the original one in place.
At which step exactly do you see that error message?
Flags: needinfo?(aswan) → needinfo?(cosmin.badescu)
I could see the error message after every loaded XPI file on Firefox 51.0a1 (20160914030200).

This issue is verified as fixed on Firefox 51.0a1 (2016-09-15), I can confirm that temporarily loading an XPI file without ID and then temporarily loading a new XPI file without ID from the same path will display both WebExtensions in about:debugging and there is no error message in the browser console.
Status: RESOLVED → VERIFIED
Flags: needinfo?(cosmin.badescu)
(In reply to CosminB from comment #17)
> This issue is verified as fixed on Firefox 51.0a1 (2016-09-15), I can
> confirm that temporarily loading an XPI file without ID and then temporarily
> loading a new XPI file without ID from the same path will display both
> WebExtensions in about:debugging and there is no error message in the
> browser console.

Wait, that's not the expected behavior -- as I said in comment 16 the expectation is that loading a new xpi file from the same path will replace the original, the first one should no longer be visible.  And when I tested this myself, that's what happened with Nightly built locally from mozilla-central.

Are you seeing something different?
Flags: needinfo?(cosmin.badescu)
I tested some cases and here are the results:
Note: The XPI files were loaded from the same path (folder).
      There were no error messages in the browser console.

XPI with Gecko ID (old) → XPI with Gecko ID (new) = both XPI files are visible (The XPI files contain a different Gecko ID)  here is a video: http://screencast.com/t/2LPADPfv

XPI with Gecko ID (old) → XPI with Gecko ID (new) = New XPI file is the only one visible (The XPI files contain the same Gecko ID)  here is a video: http://screencast.com/t/9uaHwjfENu5I

XPI without Gecko ID (old) → XPI with Gecko ID (new) = both XPI files are visible (Only New XPI file contain a Gecko ID)  here is a video: http://screencast.com/t/HIjIsXgHEznH

XPI with Gecko ID (old) → XPI without Gecko ID (new) = both XPI files are visible (Only Old  XPI file contain a Gecko ID)  here is a video: http://screencast.com/t/6pFlXvTg

XPI without Gecko ID (old) → XPI without Gecko ID (new) = both XPI files are visible (The XPI files do not contain a Gecko ID)  here is a video: http://screencast.com/t/06jxTKuS

For me these look like the correct expected results, am I wrong about this?
Flags: needinfo?(cosmin.badescu) → needinfo?(aswan)
Okay, its the very last case that I'm concerned about, but I was talking about the full path, including the file name.  You're using separate files (old.xpi vs new.xpi)
Flags: needinfo?(aswan)
(In reply to Andrew Swan [:aswan] from comment #20)
> Okay, its the very last case that I'm concerned about, but I was talking
> about the full path, including the file name.  You're using separate files
> (old.xpi vs new.xpi)

I tested the last case using a full path (including the file name),loading a new xpi file from the same path will replace the original one.
Here is a video: https://www.dropbox.com/s/qi0rd09z624se2a/Fullpathfile.gif?dl=0
Okay great, I think we're set on this bug then.
You need to log in before you can comment on or make changes to this bug.