568904 - Don't perform a shell refresh when there are no shortcuts for the installation when updating app modal id's

Status: RESOLVED FIXED

(Firefox :: Installer, defect)

Description

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

or at the very least don't perform a shell refresh when there are no shortcuts to update. Seen while running mochitest-chrome tests

Comment 1

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

Might be a good thing to not try to update the shortcut app modal id's on first run as well

Comment 2

[Jim Mathies [:jimm]]

Attachment: patch

Doesn't look like ${UnloadUAC} is needed either, as wel don't init it for this. So I just replaced Goto finish w/ Quit.

Comment 3

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

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

>diff --git a/toolkit/mozapps/installer/windows/nsis/common.nsh b/toolkit/mozapps/installer/windows/nsis/common.nsh
>--- a/toolkit/mozapps/installer/windows/nsis/common.nsh
>+++ b/toolkit/mozapps/installer/windows/nsis/common.nsh
>@@ -4675,17 +4675,17 @@
> 
>       StrCmp "$R0" "" continue +1
> 
>       ; Update this user's shortcuts with the latest app user model id.
>       ClearErrors
>       ${GetOptions} "$R0" "/UpdateShortcutAppUserModelIds" $R2
>       IfErrors hideshortcuts +1
>       ${UpdateShortcutAppModelIDs}  "$INSTDIR\${FileMainEXE}" "${AppUserModelID}"
>-      GoTo finish
>+      Quit ; no need for any shutdown.
nit: Please use the same comment as used in the other cases.
; Nothing initialized so no need to call OnEndCommon

r=me with that and thanks!

Comment 4

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

Hey Jim, just had a thought about this. I wonder if the shell keeps the previous value if the shortcut's value is changed and the shell isn't refreshed? If so, this approach won't work

Comment 5

[Jim Mathies [:jimm]]

(In reply to comment #4)
> Hey Jim, just had a thought about this. I wonder if the shell keeps the
> previous value if the shortcut's value is changed and the shell isn't
> refreshed? If so, this approach won't work

Not sure, I'll test before I land to be sure that isn't the case.

Comment 6

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

Attachment: patch - only refresh when shortcuts for this install location are found

Hey Jim, this patch makes it so the shell is only refreshed when shortcuts for this install location are found. I suspect this could be better optimized but this takes care of the case I keep running into which is running mochitest on Win7.

Comment 7

[Jim Mathies [:jimm]]

We should request blocking on one or both of these. Rob any preference?

Comment 8

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

Comment on attachment 453025 [details] [diff] [review]
patch - only refresh when shortcuts for this install location are found

Jim, I haven't had a chance to check if the first patch is safe and since I know the second one is I'll request approval for it.

Comment 9

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

Attachment: patch - ready to import

Comment 10

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

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/661fff6dec75

Comment 11

[Jim Mathies [:jimm]]

I seem to have lost my jump lists on the taskbar. Wondering if this patch might have broken something.

Comment 12

[Jim Mathies [:jimm]]

(In reply to comment #11)
> I seem to have lost my jump lists on the taskbar. Wondering if this patch might
> have broken something.

Never mind, I had jl disabled for testing bug 594821. :S Sorry for the bug spam.