1383782 - Reloading a temporary add-on from about:debugging results in an install date of "Invalid Date"

Status: RESOLVED FIXED

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

Description

[Bob Silverberg [:bsilverberg]]

I noticed this when working on ext-url-overrides and doing some interactive testing. When a temporary add-on is reloaded from about:debugging, and it contains a chrome_url_overrides.newtab manifest key, the setting is first removed from the setting store (onClose) and then added back (onManifestEntry). When the latter code runs, it calls `addon = await AddonManager.getAddonByID(id)` and at that point `addon.installDate` contains the string "Invalid Date", rather than an actual date, which is the case when the add-on is initially installed.

I'm not sure this will cause much of an issue, but I thought it best to at least report it. I can address it in the SettingsStore code, but I won't do that if it's going to be addressed in the Add-ons Manager.

Comment 1

[Andy McKay]

Bob, aswan was thinking this is something we should fix soon. Would you be able to have a look?

Comment 2

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1383782 - Record an installDate when a temporary add-on is reloaded,

Review commit: https://reviewboard.mozilla.org/r/167544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/167544/

Comment 3

[Bob Silverberg [:bsilverberg]]

I tried to do what we discussed in IRC, which was to make the installDate for the reloaded add-on the same as the installDate of the old add-on, and the updateDate for the reloaded add-on be the current time, but I couldn't get the latter to work. Simply setting updateDate at the same time I am setting installDate didn't work (i.e., the add-on didn't end up with that updateDate). It looks like updateDate is based on the date of the file that was loaded. As updateDate isn't needed by the code in question, I chose to just implement the fix for installDate for now and punt on updateDate.

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8896249 [details]
Bug 1383782 - Record an installDate when a temporary add-on is reloaded,

https://reviewboard.mozilla.org/r/167544/#review172800

::: toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js:111
(Diff revision 1)
>      AddonManager.installTemporaryAddon(addonDir),
>      promiseWebExtensionStartup(),
>    ]);
>    // The IDs should be the same.
>    equal(secondAddon.id, addon.id, "Reinstalled add-on has the expected ID");
> +  equal(secondAddon.installDate - addon.installDate, 0, "Reloaded add-on has the expected installDate.");

Why not just `equal(secondAddon.installDate, addon.installDate, ...)`

Comment 5

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8896249 [details]
Bug 1383782 - Record an installDate when a temporary add-on is reloaded,

https://reviewboard.mozilla.org/r/167544/#review172800

> Why not just `equal(secondAddon.installDate, addon.installDate, ...)`

That fails, I am guessing because the objects are not the same.

Comment 6

[Andrew Swan [:aswan]]

Comment on attachment 8896249 [details]
Bug 1383782 - Record an installDate when a temporary add-on is reloaded,

https://reviewboard.mozilla.org/r/167544/#review172800

> That fails, I am guessing because the objects are not the same.

`equal(secondAddon.installDate.valueOf(), addon.installDate.valueOf(), "...")`

Comment 7

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8896249 [details]
Bug 1383782 - Record an installDate when a temporary add-on is reloaded,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167544/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6cdbab15f8de
Record an installDate when a temporary add-on is reloaded, r=aswan

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6cdbab15f8de