171317 - Show Dialog=FALSE doesn't work

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Jeremy M. Dolan]

I made a installer preprocessor to tweak config.ini before I install a new build
every day for testing. Since I've seen the readme and license a few hundred
times now, I had it do:

-Show Dialog=TRUE
+Show Dialog=FALSE

But both are still shown.

Comment 1

[Andrew Schultz]

the "Show Dialog" is correctly parsed and mShowDialog is set properly, but it is
NEVER checked (for any of the dialog windows)

If I add:
    if (!mShowDlg)
    {
       if (aDirection==nsXInstallerDlg::FORWARD_MOVE)
           gCtx->Fdlg->Show(aDirection);
       else
           gCtx->Bdlg->Show(aDirection);
       return;
    }
to the beginning of a Show function (and make F,B=lwsci) it seems to work as
expected, but spews Gtk-Critical assertions to the terminal.

Comment 2

[Andrew Schultz]

Attachment: patch

I would have loved to have nsXXXDlg::Show() just skip on to the next dialog in
this case, but the dialogs actually depend on the installer window being in
whatever state it's usually in.  For instance, nsSetupTypeDlg::Show expects
there to be Accept/Decline buttons left over from the license.

Anyway, this patch simply goes through the motions for the welcome and license
dialogs and then goes on to the next dialog.

I'll file a new bug to rework the removal of buttons and such after each
dialog.

Comment 3

[Daniel Veditz [:dveditz]]

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

r=dveditz

Comment 4

[Andrew Schultz]

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

bug 238149 is fixed... this is easier now.

Comment 5

[Andrew Schultz]

actually, bug 238149 might have gone a bit too far as if you want to go back
over a hidden dialog window, the dialog itself doesn't know where it should go
(forward or backward).  But while writing up a patch, I'm wondering if this
makes any sense for anything other than the welcome and license dialogs.

It would work ok for the setup type dialog, but if it hits a problem (low disk
space, delete an old install) what happens?  Should it still prompt for whether
to delete the old install, or just pretend that it's in auto mode and plow
ahead?  And hiding the custom components dialog seems a bit silly.

Comment 6

[Andrew Schultz]

Attachment: patch v2

this patch implements comment 5 with the addition of handling the install
dialog.

Comment 7

[Benjamin Smedberg]

Comment on attachment 161112 [details] [diff] [review]
patch v2

I'm not comfortable reviewing this, I don't know what it does at all.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 161112 [details] [diff] [review]
patch v2

r=dveditz

Comment 9

[Andrew Schultz]

fixed

Comment 10

[:deanis74]

Comment on attachment 161112 [details] [diff] [review]
patch v2

>+    if (!mShowDlg)
>+    {
>+       gCtx->ldlg->Show();
>+       return err;
>+    }
>+   

Am I missing something?  I can't see how this works.  SKIP_DIALOG = 0 and
SHOW_DIALOG = 1, so isn't this backwards?

Comment 11

[Andrew Schultz]

the code you mentioned is in nsWelcomeDlg.  If mShowDlg = (SKIP_DIALOG=0), it
should skip to the LicenseDiaLoG.

Comment 12

[:deanis74]

Gotcha.  I mis-read the code the first time through.