Testing the upcoming NSIS-based Windows installer for Firefox 2. In the checklist of components to install in a custom install, WindowEyes does not read the current state of each item (checked or unchecked). This makes it impossible to know which components are going to be installed. Steps to reproduce: 1. Get the installer from Robert Strong. (This installer has not landed anywhere yet. This bug is *not* about the current installer in CVS.) 2. After choosing "Custom install", proceed to the "Choose Components" page. 3. WindowEyes reads "Select components to install:" label, then reads "Firefox Application, depth 1, treeview", which is annoying but not fatal. However, it does *not* read whether the item is checked or unchecked, which is fatal. 4. Arrow down to "Developer Tools". WindowEyes does not read whether the option is checked or unchecked. 5. Press Space to select the "Developer Tools" option. WindowEyes does not read that the option is now checked.

For Firefox 2.0 we might just use a custom components page with checkboxes (possibly similar to the list in the existing installer) instead of the built in components page to solve this bug.

Mark, could you test this setup.exe to see if it exhibits this bug. It has a glitch when clicking the checkbox that I should be able to fix but before I spend time on that I first need to know whether the checked state is exposed properly with this one. Thanks!

btw: it doesn't contain the app files and I suggest you exit before completing the install.

The page that has the this problem will no longer be used after DOMi is removed. If we decide to provide optional components in the future we will either need to fix this in NSIS or use a custom page to fix this bug.

The patch in bug 340173 makes it so that once DOMi is removed this page will automatically no longer be displayed.

Comment on attachment 226017 [details] Installer with modified treeview All screens look good and are spoken properly in WindowEyes 5.5G. Still has lots of high-contrast issues, but that's bug 305797. r=me

Thanks Mark, now that I am sure this is the solution to this I can look into how to solve this in the NSIS source. Due to the problems with the checkbox visual state I doubt I will get this in time for 2.0 and as long as DOMi is removed this page won't be displayed.

This page won't be displayed once DOMi is no longer included in the build - bug 339229. We should get this fixed anyways but as long as DOMi is removed this should not be a problem for Firefox 2.0.

The latest nightly build of the NSIS-based installer doesn't appear to have this patch (i.e. the custom components page isn't read properly). Is that expected, or is this a regression?

(In reply to comment #9) > The latest nightly build of the NSIS-based installer doesn't appear to have > this patch (i.e. the custom components page isn't read properly). Is that > expected, or is this a regression? That wasn't a patch... it was to verify what exactly the problem is. Also, using this method creates other problems per comment #2 just applying this change has a ui glitch with displaying the state of the checkbox. Also, rer comment #8, comment #9, and other comments this page won't be displayed in Firefox 2.0 after DOMi has been removed in bug 339229.

Does the mustfix mean that DOMi is no longer going to be removed?

Rob, I'm sure you know the answer to your question, can you update this with an ETA?

Though not ideal I think this is decent enough for Firefox 2.0. Another option would be to state DOMi will always be installed when chossing a custom install, always install DOMi and QFA (enabled) for Custom installs, and remove this page entirely.

Comment on attachment 226017 [details] Installer with modified treeview Obsoleting - a significant amount of work will need to be done to NSIS to make the treeview to fix this.

I'll attach another patch that ignores white space changes

Comment on attachment 231370 [details] [diff] [review] patch Some comments to assist with the review: >Index: browser/installer/windows/nsis/installer.nsi ... >@@ -152,19 +154,23 @@ ... >+ >+Page custom preComponents checkComponents >+ >+ > ; Select Install Components Page >-!define MUI_PAGE_CUSTOMFUNCTION_PRE preComponents >-!insertmacro MUI_PAGE_COMPONENTS >+;!define MUI_PAGE_CUSTOMFUNCTION_PRE preComponents >+;!insertmacro MUI_PAGE_COMPONENTS I'd like to leave these here commented out in hope that I am able to make the treeview accessible. >@@ -336,47 +342,64 @@ > FunctionEnd > > Section "-Application" Section1 > SectionIn 1 RO > SetDetailsPrint textonly > DetailPrint $(STATUS_CLEANUP) > SetDetailsPrint none > SetOutPath $INSTDIR >+ >+ ; Try to delete the app executable and if we can't delete it try to close the >+ ; app. This allows running an instance that is located in another directory. >+ ClearErrors >+ ${If} ${FileExists} "$INSTDIR\${FileMainEXE}" >+ ${DeleteFile} "$INSTDIR\${FileMainEXE}" >+ ${EndIf} >+ ${If} ${Errors} >+ ClearErrors >+ ${CloseApp} "true" $(WARN_APP_RUNNING_INSTALL) >+ ; Try to delete it again to prevent launching the app while we are >+ ; installing. >+ ${DeleteFile} "$INSTDIR\${FileMainEXE}" >+ ClearErrors >+ ${EndIf} >+ I moved this up from below since this should before we handle any files. What immediately follows should be commented well enough. >@@ -593,16 +616,21 @@ ... >+ StrCpy $0 "Software\Clients\StartMenuInternet\$R9\shell\properties" >+ ${WriteRegStr2} $TmpVal "$0" "" "$(OPTIONS)" 0 >+ StrCpy $0 "Software\Clients\StartMenuInternet\$R9\shell\properties\command" >+ ${WriteRegStr2} $TmpVal "$0" "" "$INSTDIR\${FileMainEXE} -preferences" 0 This adds back Options to the StarteMenuInternet and makes it so it can be localized - see bug 344738 and bug 329263 for Thunderbird. The remainder of the changes to this file are hopefully easy to understand >Index: toolkit/mozapps/installer/windows/nsis/common.nsh ... >+!macro createComponentsINI >+ WriteINIStr "$PLUGINSDIR\components.ini" "Settings" NumFields "5" >+ >+ WriteINIStr "$PLUGINSDIR\components.ini" "Field 1" Type "label" >+ WriteINIStr "$PLUGINSDIR\components.ini" "Field 1" Text "$(OPTIONAL_COMPONENTS_LABEL)" >+ WriteINIStr "$PLUGINSDIR\components.ini" "Field 1" Left "0" >+ WriteINIStr "$PLUGINSDIR\components.ini" "Field 1" Right "-1" >+ WriteINIStr "$PLUGINSDIR\components.ini" "Field 1" Top "5" >+ WriteINIStr "$PLUGINSDIR\components.ini" "Field 1" Bottom "15" >+ >+ StrCpy $R1 2 We keep track of the Field order with a var so we can handle it if DOMi or TalkBack isn't in the installer (e.g. TalkBack isn't available with hourly builds). Other than that this is essentially the same as the other ini creation macros. The rest is just white space changes.

Comment on attachment 231370 [details] [diff] [review] patch r=sspitzer, thanks for the notes in comment #17 for my benefit

Comment on attachment 231370 [details] [diff] [review] patch uir+, with a couple of string changes for clarity: > DEV_TOOLS_DESC=A tool for inspecting the DOM of HTML, XUL, and XML pages, including the browser chrome. "Inspects resources used by web pages and extensions." > QFA_DESC=A tool for submitting crash reports to Mozilla.org. "Sends information about crashes to Mozilla."

I'm going to land with these but... (In reply to comment #19) > (From update of attachment 231370 [details] [diff] [review] [edit]) > uir+, with a couple of string changes for clarity: > > > DEV_TOOLS_DESC=A tool for inspecting the DOM of HTML, XUL, and XML pages, including the browser chrome. > > "Inspects resources used by web pages and extensions." It also inspects the application and not just web pages and extensions. > > QFA_DESC=A tool for submitting crash reports to Mozilla.org. > > "Sends information about crashes to Mozilla." If this were to be changed to something along these lines I would think it would be as follows: "Sends crash information to Mozilla."

Ahhh... better still. I'll let reed change those strings in bug 346796. :)

Checked in to trunk

*grumble* Mozilla isn't what normal people consider an entity, it's an application, and while it can receive email, it won't respond unless the user clicks "reply". http://www.mozilla.org/contribute/writing/guidelines#writing-form mozilla.org is spelled in lowercase. The name of the application is Mozilla. It is a proper noun, and is therefore capitalized. The name of the organization is mozilla.org. If this is being sent to Mozilla Corporation, we should say that (and spell it however Mozilla Corporation wants to be spelled). But thank you for *not* miscasing mozilla.org, that's at least something :).

timeless, that specific string was taken from TalkBack's extension description. Also, bug 346798 will be changing it to something else anyways.

Comment on attachment 231370 [details] [diff] [review] patch a=drivers. Please land on the MOZILLA_1_8_BRANCH. Will deal with strings in a followup bug.

Checked in to MOZILLA_1_8_BRANCH