New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified date shown in about:addons is wrong - Part 2 #1778

Closed
hurda opened this Issue Jul 30, 2013 · 9 comments

Comments

Projects
None yet
4 participants
@hurda

hurda commented Jul 30, 2013

I have noticed that bug #1744 is happening again, or still happening.

Firefox 22, GM 1.11, new profile. Screenrecord here: http://videobam.com/lLVeN

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Jul 30, 2013

Collaborator

Could you please provide a written set of steps to reproduce rather than a video on a shady looking site?

Collaborator

arantius commented Jul 30, 2013

Could you please provide a written set of steps to reproduce rather than a video on a shady looking site?

@hurda

This comment has been minimized.

Show comment
Hide comment
@hurda

hurda Jul 31, 2013

Firefox 22, new profile, install GM 1.11, restart the browser.

Install any userscript from USO (i.e. http://userscripts.org/scripts/show/83584 , it also has a custom icon ), after installation, go to about:addons and double-click on the installed userscript, and the result is as expected:

1

Close the addon manager, and hit the install-button on the userscript-page again.
Then reopen the addon manager and doubleclick on the userscript. The result will be this:

2

No icon and "Last Update" will say "Thursday, January 01, 1970"
The expected result should look like the first image.

This also happens if you close and reopen the browser between the two installations.

I have no idea, why this was working fine right after #1744 was fixed, as now this errors also happens with 1.8 and earlier, which wasn't the case when I reported the bug the first time. Maybe I wasn't testing as thoroughly as I thought.

hurda commented Jul 31, 2013

Firefox 22, new profile, install GM 1.11, restart the browser.

Install any userscript from USO (i.e. http://userscripts.org/scripts/show/83584 , it also has a custom icon ), after installation, go to about:addons and double-click on the installed userscript, and the result is as expected:

1

Close the addon manager, and hit the install-button on the userscript-page again.
Then reopen the addon manager and doubleclick on the userscript. The result will be this:

2

No icon and "Last Update" will say "Thursday, January 01, 1970"
The expected result should look like the first image.

This also happens if you close and reopen the browser between the two installations.

I have no idea, why this was working fine right after #1744 was fixed, as now this errors also happens with 1.8 and earlier, which wasn't the case when I reported the bug the first time. Maybe I wasn't testing as thoroughly as I thought.

@janekptacijarabaci

This comment has been minimized.

Show comment
Hide comment
@janekptacijarabaci

janekptacijarabaci Jul 31, 2013

Contributor

Confirmed on: Greasemonkey 0.9.13+ (0.9.13 - oddly only sometimes)
Not Confirmed on: Greasemonkey 0.9.12-

List scripts on addon window - Windows (the icon):

First - icon.fileURL:
file:///C:/Users/[user]/AppData/Roaming/Mozilla/Firefox/Profiles/[id].default/gm_scripts/
YouTube_Link_Title/large.png
= OK

Second - icon.fileURL:
file:///C:/Users/[user]/AppData/Roaming/Mozilla/Firefox/Profiles/[id].default/gm_scripts/
large.png
= WRONG - missing base directory ("YouTube_Link_Title")

The same problem: http://userscripts.org/scripts/show/153699 etc.

Contributor

janekptacijarabaci commented Jul 31, 2013

Confirmed on: Greasemonkey 0.9.13+ (0.9.13 - oddly only sometimes)
Not Confirmed on: Greasemonkey 0.9.12-

List scripts on addon window - Windows (the icon):

First - icon.fileURL:
file:///C:/Users/[user]/AppData/Roaming/Mozilla/Firefox/Profiles/[id].default/gm_scripts/
YouTube_Link_Title/large.png
= OK

Second - icon.fileURL:
file:///C:/Users/[user]/AppData/Roaming/Mozilla/Firefox/Profiles/[id].default/gm_scripts/
large.png
= WRONG - missing base directory ("YouTube_Link_Title")

The same problem: http://userscripts.org/scripts/show/153699 etc.

@hurda

This comment has been minimized.

Show comment
Hide comment
@hurda

hurda Jul 31, 2013

"Confirmed on: Greasemonkey 0.9.13+ (0.9.13 - oddly only sometimes)"
I wasn't able to reproduce in .13 but in .14.

I also tested userscripts without custom icons (i.e. arantius' Linkify Plus), but since doubleclicking on the entries in the userscript-manager doesn't do anything in these older versions of GM (pre-1.4), the bug isn't as obvious because their icon isn't changing.

hurda commented Jul 31, 2013

"Confirmed on: Greasemonkey 0.9.13+ (0.9.13 - oddly only sometimes)"
I wasn't able to reproduce in .13 but in .14.

I also tested userscripts without custom icons (i.e. arantius' Linkify Plus), but since doubleclicking on the entries in the userscript-manager doesn't do anything in these older versions of GM (pre-1.4), the bug isn't as obvious because their icon isn't changing.

@Ventero

This comment has been minimized.

Show comment
Hide comment
@Ventero

Ventero Jul 31, 2013

Contributor

So the problem here seems to be that RemoteScript#install calls GM_config.install before the remote script object is completely set up (that is, before setFilename and fixTimestampsOnInstall are called). However, if GM_config.install isn't called before the script's final foldername is figured out, the old script is still installed by the time this happens. Thus, the resulting foldername will have a -1 suffix.

So I guess there's two options here: Either live with this foldername suffix, or split up the call to GM_config.install: First, uninstall the old script and copy the 'cludes over to the new script, then set up filename/install time etc., and finally call GM_config.install(this.script), which simply installs the new script without removing anything.

Contributor

Ventero commented Jul 31, 2013

So the problem here seems to be that RemoteScript#install calls GM_config.install before the remote script object is completely set up (that is, before setFilename and fixTimestampsOnInstall are called). However, if GM_config.install isn't called before the script's final foldername is figured out, the old script is still installed by the time this happens. Thus, the resulting foldername will have a -1 suffix.

So I guess there's two options here: Either live with this foldername suffix, or split up the call to GM_config.install: First, uninstall the old script and copy the 'cludes over to the new script, then set up filename/install time etc., and finally call GM_config.install(this.script), which simply installs the new script without removing anything.

@hurda

This comment has been minimized.

Show comment
Hide comment
@hurda

hurda Jul 31, 2013

When it's not showing the icon in the addon-manager, you can right-click on it, hit Edit, then just safe the script in the editor and close it. The icon and the proper "last updated"-date will be back.
The "Find Updates"-entry in its contextmenu will be greyed and deactivated, though.

hurda commented Jul 31, 2013

When it's not showing the icon in the addon-manager, you can right-click on it, hit Edit, then just safe the script in the editor and close it. The icon and the proper "last updated"-date will be back.
The "Find Updates"-entry in its contextmenu will be greyed and deactivated, though.

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Jul 31, 2013

Collaborator

The "Find Updates"-entry in its contextmenu will be greyed and deactivated, though.

This is by design, so that updates do not destroy local modifications.

Collaborator

arantius commented Jul 31, 2013

The "Find Updates"-entry in its contextmenu will be greyed and deactivated, though.

This is by design, so that updates do not destroy local modifications.

@janekptacijarabaci

This comment has been minimized.

Show comment
Hide comment
@janekptacijarabaci

janekptacijarabaci Jul 31, 2013

Contributor

The "Find Updates"-entry in its contextmenu will be greyed and deactivated, though.

...and relate it with #1743 (comment)

Contributor

janekptacijarabaci commented Jul 31, 2013

The "Find Updates"-entry in its contextmenu will be greyed and deactivated, though.

...and relate it with #1743 (comment)

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Aug 8, 2013

Collaborator

Thanks for the clear reproduction steps hurda, and diagnostics janek.

Ventero: you were basically right. The piece that needed to move was easy to move though, because config.install() is called in only one place: RemoteScript.install().

Collaborator

arantius commented Aug 8, 2013

Thanks for the clear reproduction steps hurda, and diagnostics janek.

Ventero: you were basically right. The piece that needed to move was easy to move though, because config.install() is called in only one place: RemoteScript.install().

@arantius arantius closed this in 8e2b452 Aug 8, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment