385377 - Provide optional installation for Debug QA and Palm Sync extensions on Windows.

Status: RESOLVED FIXED

Description

[Mark Banner (:standard8)]

Created attachment 269289 [details] [diff] [review]
The not-working-fix.

Currently Palm Sync (if present in an installer build) will be installed by default by the Windows NSIS installer. As it requests to install a palm sync conduit on initial start, I'd prefer it if it was an optional install.

We're also going to be getting the DebugQA extension live soon (preferably as soon as this one is fixed). This should also be an optional install.

I think (and Robert agreed on irc) that both of these shouldn't be in the standard install, but available in the custom install.

The attached patch also fixes a bug with installation of DOMI into a fresh directory - currently if the DOMI directory doesn't exist, then DOMI won't get installed.

However the attached patch doesn't work. I'm not sure why, but it doesn't appear to give the options for palmsync or debugQA (mind you my build doesn't have debugQA in anyway so that is correct), but palmsync should appear. So any ideas why it doesn't work would be appreciated.

Comment 1

[Mark Banner (:standard8)]

Just wanted to let folks know I've found the problem with this patch:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh&rev=1.15&mark=202-254#202

That common macro sets up the components page for the installer, I've fiddled about with it and got the options we need, but now I need to do a proper patch to extend it correctly for SeaMonkey (or possibly replace it as we'll want talkback at the bottom of our list of options).

Comment 2

[Mark Banner (:standard8)]

Created attachment 272550 [details] [diff] [review]
Patch v1

This patch removes talkback from the installer - as we've picked up (or very soon will be) breakpad, there's not much point to keep this code any longer - and adds optional items for DebugQA and Palm Sync.

From the "Standard Installation" point of view, DOMI is the only item included in this. DebugQA & Palm Sync are not, as I see them as more additional items.

The common.nsi that toolkit provides which creates an ini file with DOMI & Talkback isn't very useful to us. Therefore I've now created a suite specific custom.nsi file that does macros for creating the ini file, and allows us to make re-ordering the display a lot easier when components are missing from the build (optionally not built/not available on tinderbox).

Any suggestions on improving the information text are welcome.

Comment 3

[Robert Kaiser]

Umm, a bit unrelated but while we're unpackaging Talkback, could we ensure we package breakpad correctly (see also bug 388320)?

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> Umm, a bit unrelated but while we're unpackaging Talkback, could we ensure we
> package breakpad correctly (see also bug 388320)?

From the checkins/patch comparison, I think Ted's already done that for us.

Comment 5

[Robert Kaiser]

uh, right, I missed that.

Comment 6

[Frank Wein [:mcsmurf]]

Comment on attachment 272550 [details] [diff] [review]
Patch v1

>--- suite/installer/removed-files.in	8 Jul 2007 06:53:16 -0000	1.2
>+++ suite/installer/removed-files.in	16 Jul 2007 21:48:30 -0000
>@@ -1,2 +1,13 @@

Add extensions/[e-mail]/InstallDisabled to the list and stick a #ifdef XP_WIN around it.

>Index: suite/installer/windows/nsis/installer.nsi
>===================================================================
>RCS file: /cvsroot/mozilla/suite/installer/windows/nsis/installer.nsi,v
>retrieving revision 1.2
>diff -u -p -r1.2 installer.nsi
>--- suite/installer/windows/nsis/installer.nsi	29 May 2007 16:16:26 -0000	1.2
>+++ suite/installer/windows/nsis/installer.nsi	16 Jul 2007 21:48:30 -0000

I think you need to add ${If} $InstallType != 1 before this block :).

>+  ; Custom installs.
>+  ; If DOMi is installed and this install includes DOMi remove it from
>+  ; the installation directory. This will remove it if the user deselected
>+  ; DOMi on the components page.
>+  ${If} ${FileExists} "$INSTDIR\extensions\[e-mail]"
>+  ${AndIf} ${FileExists} "$EXEDIR\optional\extensions\[e-mail]"
>+    RmDir /r "$INSTDIR\extensions\[e-mail]"
>+  ${EndIf}


>@@ -908,13 +886,33 @@ Function leaveComponents
>     SectionSetFlags 1 0 ; Disable install for DOMi
>   ${EndIf}
> 
>-  ${If} ${FileExists} "$EXEDIR\optional\extensions\[e-mail]"
>+  ${If} ${FileExists} "$EXEDIR\optional\extensions\[e-mail]"
>     ${MUI_INSTALLOPTIONS_READ} $R0 "components.ini" "Field $R1" "State"
>     ; State will be 1 for checked and 0 for unchecked so we can use that to set
>     ; the section flags for installation.
>     SectionSetFlags 2 $R0
>+    IntOp $R1 $R1 + 1
>+  ${Else}
>+    SectionSetFlags 2 0 ; Disable install for debuQA
>+  ${EndIf}

s/debuQA/debugQA

>+  ${If} ${FileExists} "$EXEDIR\optional\extensions\p@m"
>+    ${MUI_INSTALLOPTIONS_READ} $R0 "components.ini" "Field $R1" "State"
>+    ; State will be 1 for checked and 0 for unchecked so we can use that to set
>+    ; the section flags for installation.
>+    SectionSetFlags 3 $R0
>+    IntOp $R1 $R1 + 1
>+  ${Else}
>+    SectionSetFlags 3 0 ; Disable install for debuQA

same as above

>+  ${EndIf}
>+
>+  ${If} ${FileExists} "$EXEDIR\optional\extensions\[e-mail]"
>+    ${MUI_INSTALLOPTIONS_READ} $R0 "components.ini" "Field $R1" "State"
>+    ; State will be 1 for checked and 0 for unchecked so we can use that to set
>+    ; the section flags for installation.
>+    SectionSetFlags 4 $R0
>   ${Else}
>-    SectionSetFlags 2 0 ; Disable install for TalkBack
>+    SectionSetFlags 4 0 ; Disable install for TalkBack
>   ${EndIf}
> FunctionEnd

r+ with the those things fixed.

Comment 7

[Mark Banner (:standard8)]

Created attachment 274281 [details] [diff] [review]
Patch v2

Incorporates comments from review of v1 and removes a section about talkback that I'd previously missed.

Comment 8

[[e-mail]]

Comment on attachment 274281 [details] [diff] [review]
Patch v2

>+!macro checkSuiteComponents
>+  ; If DOMi/Palm Sync/DebugQA aren't available skip the components page
>+  ${Unless} ${FileExists} "$EXEDIR\optional\extensions\[e-mail]"
>+  ${AndUnless} ${FileExists} "$EXEDIR\optional\extensions\[e-mail]"
>+  ${AndUnless} ${FileExists} "$EXEDIR\optional\extensions\p@m"
>+    Abort
>+  ${EndUnless}
>+!macroend
Bah, AndUnless is confusing. Which idiot invented that? :-\

>+  ${If} ${FileExists} "$EXEDIR\optional\extensions\p@m"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Type   "checkbox"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Text   "$(PALMSYNC_TITLE)"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Left   "15"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Right  "-1"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Top    "$R2"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Bottom "$R3"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" State  "1"
>+    WriteINIStr "$PLUGINSDIR\components.ini" "Field $R1" Flags  "GROUP"
>+    ${GetSize} "$EXEDIR\optional\extensions\p@m" "/S=0K" $0 $8 $9
>+    SectionSetSize 3 $0
>+    IntOp $R1 $R1 + 1
>+    IntOp $R2 $R2 + $Separation
>+    IntOp $R3 $R3 + $R4
>+  ${Else}
>+    ; Hide debugQA in the components page if it isn't available.
Palm Sync, surely?

>+  ; If DOMi doesn't exist and talkback exists then debugQA will be Field 2
talkback exists?

Comment 9

[Frank Wein [:mcsmurf]]

Comment on attachment 274281 [details] [diff] [review]
Patch v2

removed-files.in:
>+extensions/[e-mail]/install.rdf
>+extensions/[e-mail]/chrome.manifest
>+extensions/[e-mail]/components/qfaservices.dll
>+extensions/[e-mail]/components/qfaservices.xpt
>+extensions/[e-mail]/components/BrandRes.dll
>+extensions/[e-mail]/components/fullsoft.dll
>+extensions/[e-mail]/components/master.ini
>+extensions/[e-mail]/components/talkback-l10n.ini
>+extensions/[e-mail]/components/talkback.cnt
>+extensions/[e-mail]/components/talkback.exe
>+extensions/[e-mail]/components/talkback.hlp
>+#ifdef XP_WIN
>+extensions/[e-mail]/InstallDisabled
>+#endif

Oh sorry, I was not clear here. I meant #ifdef XP_WIN from extensions/[e-mail]/components/BrandRes.dll up to extensions/[e-mail]/InstallDisabled (including both files).

installer.nsi:
> Function leaveComponents
>   ; If DOMi exists then it will be Field 2.
>-  ; If DOMi doesn't exist and talkback exists then TalkBack will be Field 2 but
>-  ; if DOMi doesn't exist we won't display this page anyways.
>+  ; If DOMi doesn't exist and talkback exists then debugQA will be Field 2

s/and talkback exists//
Maybe also mention that when DOMi and debugQA do not exist, then palmsync will be Field 2.

>+  ${If} ${FileExists} "$EXEDIR\optional\extensions\p@m"
>+    ${MUI_INSTALLOPTIONS_READ} $R0 "components.ini" "Field $R1" "State"
>+    ; State will be 1 for checked and 0 for unchecked so we can use that to set
>+    ; the section flags for installation.
>+    SectionSetFlags 3 $R0
>+    IntOp $R1 $R1 + 1
>+  ${Else}
>+    SectionSetFlags 3 0 ; Disable install for debugQA

s/debugQA/palmsync

>   ${EndIf}
> FunctionEnd

r+ with that fixed.

Comment 10

[Mark Banner (:standard8)]

Patch checked in with comments addressed -> fixed.