Revision: 15822
http://sourceforge.net/p/skim-app/code/15822
Author: hofman
Date: 2025-11-24 17:19:11 +0000 (Mon, 24 Nov 2025)
Log Message:
-----------
Don't jump to next/preevious page after scrolling in horizantal mode, we cannot
trust Apple to do the correct scrolling. Fix scrolling based on what Apple has
actually done rather than what we would (reasonably) expect, as Apple isn't
reasonable.
Modified Paths:
--------------
trunk/SKBasePDFView.h
trunk/SKBasePDFView.m
Modified: trunk/SKBasePDFView.h
===================================================================
--- trunk/SKBasePDFView.h 2025-11-23 17:58:47 UTC (rev 15821)
+++ trunk/SKBasePDFView.h 2025-11-24 17:19:11 UTC (rev 15822)
@@ -44,7 +44,6 @@
@interface SKBasePDFView : PDFView {
NSInteger minHistoryIndex;
- NSInteger keyDirection;
}
- (void)colorFiltersDidChange;
Modified: trunk/SKBasePDFView.m
===================================================================
--- trunk/SKBasePDFView.m 2025-11-23 17:58:47 UTC (rev 15821)
+++ trunk/SKBasePDFView.m 2025-11-24 17:19:11 UTC (rev 15822)
@@ -390,18 +390,6 @@
[super goToDestination:destination];
}
-- (void)keyDown:(NSEvent *)event {
- unichar ch = [event firstCharacter];
- if (ch == NSPageDownFunctionKey || ch == NSDownArrowFunctionKey)
- keyDirection = 1;
- else if (ch == NSPageUpFunctionKey || ch == NSUpArrowFunctionKey)
- keyDirection = -1;
- else if (ch == SKSpaceCharacter)
- keyDirection = ([event modifierFlags] & NSEventModifierFlagShift) ? -1
: 1;
- [super keyDown:event];
- keyDirection = 0;
-}
-
// this is bound to PageDown in PDFView
- (void)scrollPageUp:(id)sender {
NSScrollView *scrollView = nil;
@@ -416,58 +404,40 @@
scrollView = [self scrollView];
clipView = [scrollView contentView];
bounds = [clipView bounds];
- } else if (hasHorizontalLayout(self) && (keyDirection == -1 ? [self
canGoToPreviousPage] : [self canGoToNextPage])) {
- clipView = [[self scrollView] contentView];
- bounds = [clipView bounds];
}
// always call super, as it also updates the current page
[super scrollPageUp:sender];
- if (scrollView) {
+ if (clipView) {
CGFloat inset = [clipView contentInsets].top;
CGFloat height = NSHeight(bounds) - inset;
CGFloat offset = fmax(height - [scrollView verticalPageScroll], 0.5 *
height);
NSRect docRect = [[scrollView documentView] frame];
- if (keyDirection == -1) {
- // check whether our assumptions are still correct
- if ([clipView isFlipped] == NO && NSMinY([clipView bounds]) >
NSMinY(bounds))
+ // check whether we have scrolled and in which direction
+ if ([clipView isFlipped] == NO) {
+ if (NSMinY([clipView bounds]) < NSMinY(bounds) - 1.0)
+ bounds.origin.y = fmax(NSMinY(docRect), NSMinY(bounds) -
offset);
+ else if (NSMinY([clipView bounds]) > NSMinY(bounds) + 1.0)
bounds.origin.y = fmin(NSMaxY(docRect) - height,
NSMinY(bounds) + offset);
- else if ([clipView isFlipped] && NSMinY([clipView bounds]) <
NSMinY(bounds))
- bounds.origin.y = fmax(NSMinY(docRect) - inset, NSMinY(bounds)
- offset);
else
return;
} else {
- // check whether our assumptions are still correct
- if ([clipView isFlipped] == NO && NSMinY([clipView bounds]) <
NSMinY(bounds))
- bounds.origin.y = fmax(NSMinY(docRect), NSMinY(bounds) -
offset);
- else if ([clipView isFlipped] && NSMinY([clipView bounds]) >
NSMinY(bounds))
+ if (NSMinY([clipView bounds]) > NSMinY(bounds) + 1.0)
bounds.origin.y = fmin(NSMaxY(docRect) - NSHeight(bounds),
NSMinY(bounds) + offset);
+ else if (NSMinY([clipView bounds]) < NSMinY(bounds) - 1.0)
+ bounds.origin.y = fmax(NSMinY(docRect) - inset, NSMinY(bounds)
- offset);
else
return;
}
[clipView scrollToPoint:bounds.origin];
[scrollView reflectScrolledClipView:clipView];
- } else if (clipView) {
- if (NSEqualPoints([clipView bounds].origin, bounds.origin)) {
- if (keyDirection == -1) {
- [self goToPreviousPage:sender];
- [self verticallyScrollToBottom];
- } else {
- [self goToNextPage:sender];
- [self verticallyScrollToTop];
- }
- }
} else if (page) {
- if (keyDirection == -1) {
- // Apple scrolls to the top of the next page rather than the bottom
- if ([[self currentPage] pageIndex] < [page pageIndex])
- [self verticallyScrollToBottom];
- } else {
- // Apple scrolls to the bottom of the previous page rather than
the top
- if ([[self currentPage] pageIndex] > [page pageIndex])
- [self verticallyScrollToTop];
- }
+ // Apple scrolls to the bottom of the previouspage or the top of the
next page rather than the bottom
+ if ([[self currentPage] pageIndex] > [page pageIndex])
+ [self verticallyScrollToTop];
+ else if ([[self currentPage] pageIndex] < [page pageIndex])
+ [self verticallyScrollToBottom];
}
}
@@ -485,130 +455,70 @@
scrollView = [self scrollView];
clipView = [scrollView contentView];
bounds = [clipView bounds];
- } else if (hasHorizontalLayout(self) && (keyDirection == 1 ? [self
canGoToNextPage] : [self canGoToPreviousPage])) {
- clipView = [[self scrollView] contentView];
- bounds = [clipView bounds];
}
// always call super, as it also updates the current page
[super scrollPageDown:sender];
- if (scrollView) {
+ if (clipView) {
CGFloat inset = [clipView contentInsets].top;
CGFloat height = NSHeight(bounds) - inset;
CGFloat offset = fmax(height - [scrollView verticalPageScroll], 0.5 *
height);
NSRect docRect = [[scrollView documentView] frame];
- if (keyDirection == 1) {
- // check whether our assumptions are still correct
- if ([clipView isFlipped] == NO && NSMinY([clipView bounds]) <
NSMinY(bounds))
+ // check whether we have scrolled and in which direction
+ if ([clipView isFlipped] == NO) {
+ if (NSMinY([clipView bounds]) > NSMinY(bounds) + 1.0)
+ bounds.origin.y = fmin(NSMaxY(docRect) - height,
NSMinY(bounds) + offset);
+ else if (NSMinY([clipView bounds]) < NSMinY(bounds) - 1.0)
bounds.origin.y = fmax(NSMinY(docRect), NSMinY(bounds) -
offset);
- else if ([clipView isFlipped] && NSMinY([clipView bounds]) >
NSMinY(bounds))
- bounds.origin.y = fmin(NSMaxY(docRect) - NSHeight(bounds),
NSMinY(bounds) + offset);
else
return;
} else {
- // check whether our assumptions are still correct
- if ([clipView isFlipped] == NO && NSMinY([clipView bounds]) >
NSMinY(bounds))
- bounds.origin.y = fmin(NSMaxY(docRect) - height,
NSMinY(bounds) + offset);
- else if ([clipView isFlipped] && NSMinY([clipView bounds]) <
NSMinY(bounds))
+ if (NSMinY([clipView bounds]) < NSMinY(bounds) + 1.0)
bounds.origin.y = fmax(NSMinY(docRect) - inset, NSMinY(bounds)
- offset);
+ else if (NSMinY([clipView bounds]) > NSMinY(bounds) - 1.0)
+ bounds.origin.y = fmin(NSMaxY(docRect) - NSHeight(bounds),
NSMinY(bounds) + offset);
else
return;
}
[clipView scrollToPoint:bounds.origin];
[scrollView reflectScrolledClipView:clipView];
- } else if (clipView) {
- if (NSEqualPoints([clipView bounds].origin, bounds.origin)) {
- if (keyDirection == 1) {
- [self goToNextPage:sender];
- [self verticallyScrollToTop];
- } else {
- [self goToPreviousPage:sender];
- [self verticallyScrollToBottom];
- }
- }
} else if (page) {
- if (keyDirection == 1) {
- // Apple scrolls to the bottom of the previous page rather than
the top
- if ([[self currentPage] pageIndex] > [page pageIndex])
- [self verticallyScrollToTop];
- } else {
- // Apple scrolls to the top of the next page rather than the bottom
- if ([[self currentPage] pageIndex] < [page pageIndex])
- [self verticallyScrollToBottom];
- }
+ // Apple scrolls to the top of the next page or the bottom of the
previous page rather than the bottom
+ if ([[self currentPage] pageIndex] < [page pageIndex])
+ [self verticallyScrollToBottom];
+ else if ([[self currentPage] pageIndex] > [page pageIndex])
+ [self verticallyScrollToTop];
}
}
// this is bound to ArrowDown in PDFView
- (void)scrollLineUp:(id)sender {
- NSClipView *clipView = nil;
- NSRect bounds = NSZeroRect;
- PDFPage *page = nil;
+ PDFPage *page = ([self displayMode] & kPDFDisplaySinglePageContinuous) ?
nil : [self currentPage];
- if (([self displayMode] & kPDFDisplaySinglePageContinuous) == 0) {
- page = [self currentPage];
- } else if (hasHorizontalLayout(self) && (keyDirection == -1 ? [self
canGoToPreviousPage] : [self canGoToNextPage])) {
- clipView = [[self scrollView] contentView];
- bounds = [clipView bounds];
- }
-
[super scrollLineUp:sender];
- if (clipView && NSEqualPoints([clipView bounds].origin, bounds.origin)) {
- if (keyDirection == -1) {
- [self goToPreviousPage:sender];
+ if (page) {
+ // Apple scrolls to the bottom of the previouspage or the top of the
next page rather than the bottom
+ if ([[self currentPage] pageIndex] > [page pageIndex])
+ [self verticallyScrollToTop];
+ else if ([[self currentPage] pageIndex] < [page pageIndex])
[self verticallyScrollToBottom];
- } else {
- [self goToNextPage:sender];
- [self verticallyScrollToTop];
- }
- } else if (page) {
- if (keyDirection == -1) {
- // Apple scrolls to the top of the previous page rather than the
bottom
- if ([[self currentPage] pageIndex] < [page pageIndex])
- [self verticallyScrollToBottom];
- } else {
- // Apple scrolls to the bottom of the next page rather than the top
- if ([[self currentPage] pageIndex] > [page pageIndex])
- [self verticallyScrollToTop];
- }
}
}
// this is bound to ArrowUp in PDFView
- (void)scrollLineDown:(id)sender {
- NSClipView *clipView = nil;
- NSRect bounds = NSZeroRect;
- PDFPage *page = nil;
+ PDFPage *page = ([self displayMode] & kPDFDisplaySinglePageContinuous) ?
nil : [self currentPage];
- if (([self displayMode] & kPDFDisplaySinglePageContinuous) == 0) {
- page = [self currentPage];
- } else if (hasHorizontalLayout(self) && (keyDirection == 1 ? [self
canGoToNextPage] : [self canGoToPreviousPage])) {
- clipView = [[self scrollView] contentView];
- bounds = [clipView bounds];
- }
-
[super scrollLineDown:sender];
- if (clipView && NSEqualPoints([clipView bounds].origin, bounds.origin)) {
- if (keyDirection == 1) {
- [self goToNextPage:sender];
+ if (page) {
+ // Apple scrolls to the bottom of the previouspage or the top of the
next page rather than the bottom
+ if ([[self currentPage] pageIndex] > [page pageIndex])
[self verticallyScrollToTop];
- } else {
- [self goToPreviousPage:sender];
+ else if ([[self currentPage] pageIndex] < [page pageIndex])
[self verticallyScrollToBottom];
- }
- } else if (page) {
- if (keyDirection == 1) {
- // Apple scrolls to the bottom of the next page rather than the top
- if ([[self currentPage] pageIndex] > [page pageIndex])
- [self verticallyScrollToTop];
- } else {
- // Apple scrolls to the top of the previous page rather than the
bottom
- if ([[self currentPage] pageIndex] < [page pageIndex])
- [self verticallyScrollToBottom];
- }
}
}
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
_______________________________________________
Skim-app-commit mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/skim-app-commit
