Merge pull request #2323 from mrcarlberg/scroll_rect_to_visible_with_large_rect

Fixed: Method scrollRectToVisible:(CGRect)aRect in CPView didn’t work...

AppKit/CPView.j

@@ -2731,18 +2731,47 @@ setBoundsOrigin:
     if (CGRectContainsRect(documentViewVisibleRect, rectInDocumentView))
         return NO;
 
-    var scrollPoint = CGPointMakeCopy(documentViewVisibleRect.origin);
+    var currentScrollPoint = documentViewVisibleRect.origin,
+        scrollPoint = CGPointMakeCopy(currentScrollPoint),
+        rectInDocumentViewMinX = CGRectGetMinX(rectInDocumentView),
+        documentViewVisibleRectMinX = CGRectGetMinX(documentViewVisibleRect),
+        doesItFitForWidth = documentViewVisibleRect.size.width >= rectInDocumentView.size.width;
 
     // One of the following has to be true since our current visible rect didn't contain aRect.
-    if (CGRectGetMinX(rectInDocumentView) < CGRectGetMinX(documentViewVisibleRect))
-        scrollPoint.x = CGRectGetMinX(rectInDocumentView);
-    else if (CGRectGetMaxX(rectInDocumentView) > CGRectGetMaxX(documentViewVisibleRect))
-        scrollPoint.x += CGRectGetMaxX(rectInDocumentView) - CGRectGetMaxX(documentViewVisibleRect);
+    if (rectInDocumentViewMinX < documentViewVisibleRectMinX && doesItFitForWidth)
+        // Scroll to left edge of aRect as it is to the left of the visible rect and it fit inside
+        scrollPoint.x = rectInDocumentViewMinX;
+    else if (CGRectGetMaxX(rectInDocumentView) > CGRectGetMaxX(documentViewVisibleRect) && doesItFitForWidth)
+        // Scroll to right edge of aRect as it is to the right of the visible rect and it fit inside
+        scrollPoint.x = CGRectGetMaxX(rectInDocumentView) - documentViewVisibleRect.size.width;
+    else if (rectInDocumentViewMinX > documentViewVisibleRectMinX)
+        // Scroll to left edge of aRect as it is to the right of the visible rect and it doesn't fit inside
+        scrollPoint.x = rectInDocumentViewMinX;
+    else if (CGRectGetMaxX(rectInDocumentView) < CGRectGetMaxX(documentViewVisibleRect))
+        // Scroll to right edge of aRect as it is to the left of the visible rect and it doesn't fit inside
+        scrollPoint.x = CGRectGetMaxX(rectInDocumentView) - documentViewVisibleRect.size.width;
 
-    if (CGRectGetMinY(rectInDocumentView) < CGRectGetMinY(documentViewVisibleRect))
-        scrollPoint.y = CGRectGetMinY(rectInDocumentView);
-    else if (CGRectGetMaxY(rectInDocumentView) > CGRectGetMaxY(documentViewVisibleRect))
-        scrollPoint.y += CGRectGetMaxY(rectInDocumentView) - CGRectGetMaxY(documentViewVisibleRect);
+    var rectInDocumentViewMinY = CGRectGetMinY(rectInDocumentView),
+        documentViewVisibleRectMinY = CGRectGetMinY(documentViewVisibleRect),
+        doesItFitForHeight = documentViewVisibleRect.size.height >= rectInDocumentView.size.height;
+
+    if (rectInDocumentViewMinY < documentViewVisibleRectMinY && doesItFitForHeight)
+        // Scroll to top edge of aRect as it is above the visible rect and it fit inside
+        scrollPoint.y = rectInDocumentViewMinY;
+    else if (CGRectGetMaxY(rectInDocumentView) > CGRectGetMaxY(documentViewVisibleRect) && doesItFitForHeight)
+        // Scroll to bottom edge of aRect as it is below the visible rect and it fit inside
+        scrollPoint.y = CGRectGetMaxY(rectInDocumentView) - documentViewVisibleRect.size.height;
+    else if (rectInDocumentViewMinY > documentViewVisibleRectMinY)
+        // Scroll to top edge of aRect as it is below the visible rect and it doesn't fit inside
+        scrollPoint.y = rectInDocumentViewMinY;
+    else if (CGRectGetMaxY(rectInDocumentView) < CGRectGetMaxY(documentViewVisibleRect))
+        // Scroll to bottom edge of aRect as it is above the visible rect and it doesn't fit inside
+        scrollPoint.y = CGRectGetMaxY(rectInDocumentView) - documentViewVisibleRect.size.height;
+
+    // Don't scroll if aRect contains the whole visible rect as it is already as visible as possible.
+    // We check this by comparing to new scrollPoint to the current.
+    if (CGPointEqualToPoint(scrollPoint, currentScrollPoint))
+        return NO;
 
     [enclosingClipView scrollToPoint:scrollPoint];
 

Tests/AppKit/CPScrollViewTest.j

@@ -229,7 +229,7 @@
 
     [scrollView setDocumentView:documentView];
 
-    [textField1 setFrameOrigin:CGPointMake(0, 0)];
+    [textField1 setFrameOrigin:CGPointMake(10, 10)];
     [textField2 setFrameOrigin:CGPointMake(500, 500)];
 
     [documentView addSubview:textField1];
@@ -242,9 +242,11 @@
     [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner"];
 
     // Make the second text field visible
-    [textField2 scrollRectToVisible:[textField2 bounds]];
+    var hasScrolled = [textField2 scrollRectToVisible:[textField2 bounds]];
 
-    var visibleRectOriginShouldBeAt = CGPointMake(500 - originalVisibleSize.width + textField2Size.width, 500 -originalVisibleSize.height + textField2Size.height);
+    [self assertTrue:hasScrolled];
+
+    var visibleRectOriginShouldBeAt = CGPointMake(500 - originalVisibleSize.width + textField2Size.width, 500 - originalVisibleSize.height + textField2Size.height);
 
     visibleRect = [documentView visibleRect];
 
@@ -252,12 +254,66 @@
     [self assertPoint:visibleRectOriginShouldBeAt equals:visibleRect.origin message:@"Second text field not at lower right corner in visible rect"];
 
     // Make the first text field visible again
-    [textField1 scrollRectToVisible:[textField2 bounds]];
+    hasScrolled = [textField1 scrollRectToVisible:[textField1 bounds]];
+    [self assertTrue:hasScrolled];
 
     visibleRect = [documentView visibleRect];
 
     // We should now be back at top left corner
-    [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"];
+    [self assertPoint:CGPointMake(10, 10) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"];
+
+    // Try to scroll again and it should not scroll
+    hasScrolled = [textField1 scrollRectToVisible:[textField1 bounds]];
+    [self assertFalse:hasScrolled];
+}
+
+- (void)testScrollRectToVisibleWithLargeRect
+{
+    var scrollView = [[CPScrollView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)],
+        documentView = [[CPView alloc] initWithFrame:CGRectMake(0, 0, 1000, 1000)],
+        view1 = [[CPView alloc] initWithFrame:CGRectMake(0, 0, 200, 200)],
+        view2 = [[CPView alloc] initWithFrame:CGRectMake(500, 500, 200, 200)],
+        view1Size = CGSizeMakeCopy([view1 bounds].size),
+        view2Size = CGSizeMakeCopy([view2 bounds].size);
+
+    [scrollView setDocumentView:documentView];
+
+    [view1 setFrameOrigin:CGPointMake(0, 0)];
+    [view2 setFrameOrigin:CGPointMake(500, 500)];
+
+    [documentView addSubview:view1];
+    [documentView addSubview:view2];
+
+    var visibleRect = [documentView visibleRect],
+        originalVisibleSize = CGSizeMakeCopy(visibleRect.size);
+
+    // Make sure we are at the top left corner
+    [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner"];
+
+    // Make the second view visible
+    var hasScrolled = [view2 scrollRectToVisible:[view2 bounds]];
+
+    [self assertTrue:hasScrolled];
+
+    visibleRect = [documentView visibleRect];
+
+    // We should now have the origin of view2 in the upper left corner
+    [self assertPoint:CGPointMake(500, 500) equals:visibleRect.origin message:@"Origin of second view not at upper left corner in visible rect"];
+
+    // Make the first view  visible again
+    hasScrolled = [view1 scrollRectToVisible:[view1 bounds]];
+    [self assertTrue:hasScrolled];
+
+    visibleRect = [documentView visibleRect];
+
+    var visibleRectOriginShouldBeAt = CGPointMake(200 - visibleRect.size.width, 200 - visibleRect.size.height);
+
+    // We should now be back almost at top left corner except that the lower right corner of the rect should be at the lower right corner of the visible rect.
+    [self assertPoint:visibleRectOriginShouldBeAt equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"];
+
+    // Try to scroll again and it should not scroll even as some parts are outside the visible rect
+    hasScrolled = [view1 scrollRectToVisible:[view1 bounds]];
+    [self assertFalse:hasScrolled];
 }
 
 -(void)testNotificationsRegistered