DistributedProofreaders / guiguts

Perl/Tk text editor designed for editing and formatting public domain material for inclusion at Project Gutenberg
GNU General Public License v2.0
9 stars 10 forks source link

Prev/next page buttons malfunction after setting and changing Page Lbl #1269

Closed charliehoward4dp closed 1 year ago

charliehoward4dp commented 1 year ago

If this is a bug, rather than a user error, it occurs in every release at least as far back as 1.4.0 (Edit: 1.0.25), so it wasn't caused by the relatively recent change involving the '<' and '>' buttons.

  1. Open any file for which page numbers ("Lbl") have been set. The attached file is as good as any, but in my tests, it happened with over a dozen files, and never worked properly.
  2. Click "Lbl:" and go to page 150. That will work.
  3. Click the less-than arrow to the left of "See Img" a couple of times, and the pages will move down properly.
  4. Click "Lbl:" again and go to page 100. That will work.
  5. Click the less-than arrow to the left of "See Img" and instead of going to page 99, it goes to page 147 (or whatever was the next lower page after doing step 3).
  6. The only way I've found to even temporarily fix the problem is to close/reopen GG.
  7. The area around the page you start at in step 3 becomes an anchor, in a manner of speaking. That is, if in step 3, you went to page 100, then to 150, then used less-than, it'll go to page 99, not to 149.

The .bin file looks fine.

Update: This also happens if only the "Img:" button and the less-than / greater-than buttons are used: click "Img" and enter a number, use less-than, then click "Img" again and enter a different number, and use less-than or greater-than.

pagenumbug.zip

windymilla commented 1 year ago

Prior to 2 months ago (so all released versions) the Page Marker Adjustment dialog was popped when you used the < and > buttons to "See prev/next image". The code looks in the Page Marker Adjustment dialog to see which is the "current" page marker, and then goes back/forward from there. Since the edit 2 months ago, (i.e. post 1.6.0 release) the Adjustment dialog is no longer popped, and (you guessed it) therefore stumbles over an (obscure) bug I introduced so that you could go back/forwards without the Adjustment dialog being popped. Just to amuse anyone who reads these comments, I changed my $num = $::lglobal{pagenumentry}->get; to my $num = $::lglobal{pagenumentry}->get if Tk::Exists( $::lglobal{pagenumentry} ); to avoid an error when the Adjustment dialog wasn't there.

According to the Perl docs:

NOTE: The behaviour of a my, state, or our modified with a statement modifier conditional or loop construct (for example, my $x if ...) is undefined. The value of the my variable may be undef, any previously assigned value, or possibly anything else. Don't rely on it. Future versions of perl might do something different from the version of perl you try it out on. Here be dragons.

Looks like the dragons got me! The supposedly local temporary variable retained it's value from one call of the routine to the next (giving the results Charlie spotted). A comment online said that what currently happens is that the variable gets declared at compile time, and then doesn't get redeclared anew at run time because of the if, hence the variable persists rather than being undefined as I expected, and relied on.

charliehoward4dp commented 1 year ago

Then, there seem to be two issues: the recent one you described above, and the one that's been in every GG release since at least 1.0.25. As I said in the (much-edited) original post, this problem occurs in all of them, not just in 1.6.0 and later.

windymilla commented 1 year ago

That behavior is deliberate though, whereas mine wasn't. If you have the Adjustment dialog visible (which you generally will if you're using older versions, even if it's hidden behind the main window) then it will go forward or back one page from the one shown in that dialog. The < and > work the same as Previous/Next Marker buttons in the Adjustment dialog. As I say, I'm sure that is deliberate, hence why it has been there for ever, but having made the code not rely on that dialog, I introduced the bug you spotted. We could make the < and > work differently from Previous/Next Marker if you think that would be better/clearer - I think it would. So, < and > would go forwards/backwards from the current cursor position, whereas the buttons in the dialog would go forwards/backwards from the marker shown in the dialog. That would seem logical.