Closed Bug 171317 Opened 22 years ago Closed 20 years ago

Show Dialog=FALSE doesn't work

Categories

(SeaMonkey :: Installer, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmd, Assigned: ajschult784)

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
QA Contact: bugzilla → ktrina
Attached patch patch (obsolete) — Splinter Review
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.
Assignee: dveditz+bmo → ajschult
Status: NEW → ASSIGNED
Attachment #144434 - Flags: review?(dveditz+bmo)
Comment on attachment 144434 [details] [diff] [review]
patch

r=dveditz
Attachment #144434 - Flags: review?(dveditz) → review+
Comment on attachment 144434 [details] [diff] [review]
patch

bug 238149 is fixed... this is easier now.
Attachment #144434 - Attachment is obsolete: true
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.
Attached patch patch v2Splinter Review
this patch implements comment 5 with the addition of handling the install
dialog.
Attachment #161112 - Flags: review?(bsmedberg)
Comment on attachment 161112 [details] [diff] [review]
patch v2

I'm not comfortable reviewing this, I don't know what it does at all.
Attachment #161112 - Flags: review?(bsmedberg) → review?(dveditz)
Comment on attachment 161112 [details] [diff] [review]
patch v2

r=dveditz
Attachment #161112 - Flags: review?(dveditz) → review+
Attachment #161112 - Flags: superreview?(bryner)
Product: Browser → Seamonkey
Attachment #161112 - Flags: superreview?(bryner) → superreview+
fixed
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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?
the code you mentioned is in nsWelcomeDlg.  If mShowDlg = (SKIP_DIALOG=0), it
should skip to the LicenseDiaLoG.
Gotcha.  I mis-read the code the first time through.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: