Skip Menu |

This queue is for tickets about the Tk-Wizard CPAN distribution.

Report information
The Basics
Id: 37333
Status: resolved
Worked: 40 min
Priority: 0/
Queue: Tk-Wizard

People
Owner: LGODDARD [...] cpan.org
Requestors: geert.depaep [...] uptime.be
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: (no value)



Subject: Pre/post nextButtonAction does not work as intended when associated function returns false
I want to create a preNextButtonAction function that performs some checks using the info entered on the current wizard screen. When the checks fail, the wizard should not allow you to continue to the next page. I suppose that is the intended behaviour of a preNextButtonAction routine. However when the function I associate to the nextbuttonaction returns false, all is messed up, and it is impossible to use the Next button again, even when the function returns true later on. The issue refers to the code in sub _NextButtonEventCycle of Wizard.pm. Obviously it holds a counter how many times it has been called at the same time (the variable "_inside_nextButtonEventCycle_" which I will refer to as "counter"). The counter is increased first. However when its value is more than 1, the routine exits. I assume that is done to avoid that the routine is running twice at the same time?... Suppose however that the routine is run only once. The next step in the code is to execute (_dispatch) the preNextButtonAction. When this returns 0, a "return" is called and the _NextButtonEventCycle is left. According to me, you first have to decrease the counter again. Otherwise its value remains at 1, and when you click the Next button again, you will always immediately exit the _NextButtonEventCycle because the counter will always increase to 2. So the code has to become: if ( _dispatch( $self->cget( -preNextButtonAction ) ) ) { DEBUG "preNextButtonAction says we should not go ahead"; # GDP: Decrease _inside_nextButtonEventCycle_ again # otherwise unable to click Next button again # (will always bail out on condition above. $self->{_inside_nextButtonEventCycle_}--; return; } The second issue refers to the postNextButtonAction. According to me, it makes no sense to leave the _NextButtonEventCycle when that routine returns false. At the moment when you call it, the page counter has been increased already, the next page has already been rendered and it is impossible to go back to the previous page again. I would just execute the routine and continue, no matter wheter it succeeds or fails. Besides, leaving the _NextButtonEventCycle without decreasing _inside_nextButtonEventCycle_ would generate the same problem as above. So I would put the "return" statement in comment: DEBUG "Before _dispatch postNextButtonAction"; if ( _dispatch( $self->cget( -postNextButtonAction ) ) ) { DEBUG "postNextButtonAction says we should not go ahead"; # GDP: Makes no sense to leave routine here. # Page counter is already increased anyway. # Besides, we have to decrease _inside_nextButtonEventCycle_ anyway. # So putting next line in comment. # return; } It might be useful to populate a variable in the $self so you can query afterwards if this routine had succeeded or not, something like $self->lastNextButtonActionSucceeded() or so. The file I have uploaded has an example that demonstrates the problem. Run it in the current version of Wizard.pm, and press the next button on page 2 while the radio button is in the first state. You will see that then the Next button becomes unusable.
Subject: effe2.pl
use Tk; use Tk::Wizard (); use Log::Log4perl qw(:easy); Log::Log4perl->easy_init($DEBUG); my $wizard = Tk::Wizard->new( ); my $RadioChoice = 'N'; $wizard->addPage( sub {$wizard->blank_frame(-title => "page 1"); }, -preNextButtonAction=>sub{1} ); $wizard->addPage( sub {$Frame1 = $wizard->blank_frame(-title => "page 2"); $Frame1->Radiobutton(-text => "Cannot goto next page. Choosing this one will make Next button unusable", -variable => \$RadioChoice, -value => 'N')->pack(); $Frame1->Radiobutton(-text => "Choose this one for going to next page", -variable => \$RadioChoice, -value => 'E')->pack(); return $Frame1; }, -preNextButtonAction => sub { if ($RadioChoice eq 'E') {return 1;} else {return 0;} }, ); $wizard->addPage( sub {$wizard->blank_frame(-title => "page 3"); }, -preNextButtonAction=>sub{1} ); $wizard->addPage( sub {$wizard->blank_frame(-title => "page 4"); }, ); $wizard->Show; MainLoop; exit;
Wow, good catch, thank you! In the preNext, I think rather than decrementing the counter, it should be set to zero. I.e., when the preNext routine determines that it's not safe to advance, the counter should be set to zero (otherwise, any pending "next" operations might end up forcing it to advance). In the postNext, I think it is too late to stop the page advance, so maybe the callback's return value should just be ignored in any case... This will of course need further thought and testing... - - Martin
From: geert.depaep [...] uptime.be
Thanks for your quick reply. Could you please explain me why the concept of the counter is introduced? Is it when the enduser clicks twice on the 'next' button quickly? Could this routine be entered twice at the same time? Or are there other cases that you have to handle with this counter?
Subject: RE: [rt.cpan.org #37333] Pre/post nextButtonAction does not work as intended when associated function returns false
Date: Wed, 02 Jul 2008 18:24:34 -0400
To: <bug-Tk-Wizard [...] rt.cpan.org>, "'undisclosed-recipients:'"
From: "Martin Thurn" <mthurn [...] verizon.net>
The counter was actually introduced so that the automated tests can progress at extremely high speed. The test programs basically simulate clicking the Next button at super-human speed; the counter is needed to prevent the callback function from being interrupted by another instance of itself in an inopportune moment, which would cause everything to get all gummed up. And yes, it would also cover the case where a user clicks the button too fast 8-) - - Martin
RT-Send-CC: MTHURN [...] CPAN.ORG
Hi both, and thank you very much for your help with spotting and solving this. I've uploaded a dev release, Tk-Wizard-2.138_01.tar.gz, which impliments the -- solution discussed elsewhere in this ticket's log, as the most logical solution for the moment. Please take a moment to test: it's been a long day and I'm not convinced. If I don't hear anything in a day or so, and if the Tk::JPEG issue gets resolved, I shall upload a public version, 2.138. Cheers Lee
From: geert.depaep [...] uptime.be
Thanks. I would like to look at it, however I am totally unfamiliar with cpan development. I don't know where I can preview your latest release. I see some info about a 'pause' server, but I have no login on that at the moment, and the readme says it takes about a week to confirm a new registration. Or is there another location where I can preview the new candidate release?
Subject: Re: [rt.cpan.org #37333] Pre/post nextButtonAction does not work as intended when associated function returns false
Date: Fri, 04 Jul 2008 12:07:11 +0200
To: bug-Tk-Wizard [...] rt.cpan.org
From: Lee Goddard <leegee [...] gmail.com>
Geert De Paep via RT wrote: Show quoted text
> Queue: Tk-Wizard > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=37333 > > > Thanks. I would like to look at it, however I am totally unfamiliar with > cpan development. I don't know where I can preview your latest release. > I see some info about a 'pause' server, but I have no login on that at > the moment, and the readme says it takes about a week to confirm a new > registration. Or is there another location where I can preview the new > candidate release?
Ah, very sorry! PAUSE tells me it will end up as follows: I'll e-mail the tar ball to you directly, if you wish. The uploaded file Tk-Wizard-2.138_01.tar.gz has entered CPAN as file: $CPAN/authors/id/L/LG/LGODDARD/Tk-Wizard-2.138_01.tar.gz size: 821123 bytes md5: b60c0f459e1b2fa6de10c258f3499b4d No action is required on your part Request entered by: LGODDARD (Lee ♫ Goddard) Request entered on: Thu, 03 Jul 2008 19:42:02 GMT Request completed: Thu, 03 Jul 2008 19:43:15 GMT Thanks, -- paused, v1016
From: geert.depaep [...] uptime.be
Yes please. Mail to geert.depaep(at)uptime.be
Before I moved country, I tried to mail you and it got bounced back with a complaint about spam/virus worries. I informed you of this, but have heard nothing back -- did I miss your reply or have you been too busy to test? Is there anything else I can do to help?
Subject: RE: [rt.cpan.org #37333] Pre/post nextButtonAction does not work as intended when associated function returns false
Date: Tue, 9 Sep 2008 10:35:58 +0200
To: "bug-Tk-Wizard [...] rt.cpan.org" <bug-Tk-Wizard [...] rt.cpan.org>
From: De Paep Geert <Geert.DePaep [...] uptime.be>
Hi Lee, I didn't receive your previous mails. Anyway, the changes you made were ok for me. Thanks, Geert Show quoted text
-----Original Message----- From: Lee Goddard via RT [mailto:bug-Tk-Wizard@rt.cpan.org] Sent: vrijdag 5 september 2008 18:55 To: De Paep Geert Subject: [rt.cpan.org #37333] Pre/post nextButtonAction does not work as intended when associated function returns false <URL: http://rt.cpan.org/Ticket/Display.html?id=37333 > Before I moved country, I tried to mail you and it got bounced back with a complaint about spam/virus worries. I informed you of this, but have heard nothing back -- did I miss your reply or have you been too busy to test? Is there anything else I can do to help?
Subject: Re: [rt.cpan.org #37333] Pre/post nextButtonAction does not work as intended when associated function returns false
Date: Tue, 09 Sep 2008 10:53:27 +0200
To: bug-Tk-Wizard [...] rt.cpan.org
From: Lee Goddard <leegee [...] gmail.com>
Thanks, Geert. I'll make the dev version 'real' soon. Cheers Lee Geert De Paep via RT wrote: Show quoted text
> Queue: Tk-Wizard > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=37333 > > > Hi Lee, > I didn't receive your previous mails. Anyway, the changes you made were ok for me. > Thanks, > Geert > > -----Original Message----- > From: Lee Goddard via RT [mailto:bug-Tk-Wizard@rt.cpan.org] > Sent: vrijdag 5 september 2008 18:55 > To: De Paep Geert > Subject: [rt.cpan.org #37333] Pre/post nextButtonAction does not work as intended when associated function returns false > > <URL: http://rt.cpan.org/Ticket/Display.html?id=37333 > > > Before I moved country, I tried to mail you and it got bounced back > with a complaint about spam/virus worries. I informed you of this, but > have heard nothing back -- did I miss your reply or have you been too > busy to test? Is there anything else I can do to help? > > > >
No further reports that this is an issue, so assuming it to be fixed by the patch.