WebCore:
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Sep 2004 14:16:36 +0000 (14:16 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Sep 2004 14:16:36 +0000 (14:16 +0000)
        Reviewed by Hyatt

        Fix for this bug:

        <rdar://problem/3818296> REGRESSION (Mail): centerSelectionInVisibleArea does not work correctly

        * kwq/KWQKHTMLPart.h:
        * kwq/KWQKHTMLPart.mm:
        (KWQKHTMLPart::centerSelectionInVisibleArea): New function. Handles both caret
        and range selections correctly.
        * kwq/KWQNSViewExtras.h: Add forceCentering boolean to some methods in this file.
        * kwq/KWQNSViewExtras.m: Ditto. This addition has been done since the AppKit
        method we use to do the centering, -[NSView scrollRectToVisible:],  does not alter
        the view if the rectangle passed to it is already in view. When forceCentering is
        true, extra math is done to make scrollRectToVisible center the rectangle we want.
        (-[NSView _KWQ_scrollFrameToVisible]): Pass NO for forceCentering in call through to
        _KWQ_scrollRectToVisible:forceCentering:
        (-[NSView _KWQ_scrollRectToVisible:forceCentering:]): Add forceCentering argument.
        (-[NSView _KWQ_scrollRectToVisible:inView:forceCentering:]): Ditto.
        (-[NSClipView _KWQ_scrollRectToVisible:inView:forceCentering:]): Ditto. Do extra
        math to implement the forceCentering effect.
        * kwq/KWQScrollView.h: Add forceCentering default argument to ensureRectVisibleCentered.
        * kwq/KWQScrollView.mm:
        (QScrollView::ensureRectVisibleCentered): Ditto.
        * kwq/WebCoreBridge.h:
        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge centerSelectionInVisibleArea]): New function. Call through to KWQKHTMLPart.

WebKit:

        Reviewed by Hyatt

        Fix for this bug:

        <rdar://problem/3818296> REGRESSION (Mail): centerSelectionInVisibleArea does not work correctly

        * WebView.subproj/WebHTMLView.m:
        (-[WebHTMLView centerSelectionInVisibleArea:]): Now calls new centerSelectionInVisibleArea
        bridge function instead of ensureCaretVisible. Now handles caret selections and range
        selections correctly.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@7720 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog-2005-08-23
WebCore/kwq/KWQKHTMLPart.h
WebCore/kwq/KWQKHTMLPart.mm
WebCore/kwq/KWQNSViewExtras.h
WebCore/kwq/KWQNSViewExtras.m
WebCore/kwq/KWQScrollView.h
WebCore/kwq/KWQScrollView.mm
WebCore/kwq/WebCoreBridge.h
WebCore/kwq/WebCoreBridge.mm
WebKit/ChangeLog
WebKit/WebView.subproj/WebHTMLView.m

index 0f7cb7dff15006e0223f5d69e211fab51cb8adae..117beda0c14045f2ce5addc3839982c9937a8a76 100644 (file)
@@ -1,3 +1,33 @@
+2004-09-29  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by Hyatt
+        
+        Fix for this bug:
+        
+        <rdar://problem/3818296> REGRESSION (Mail): centerSelectionInVisibleArea does not work correctly
+        
+        * kwq/KWQKHTMLPart.h:
+        * kwq/KWQKHTMLPart.mm:
+        (KWQKHTMLPart::centerSelectionInVisibleArea): New function. Handles both caret
+        and range selections correctly.
+        * kwq/KWQNSViewExtras.h: Add forceCentering boolean to some methods in this file.
+        * kwq/KWQNSViewExtras.m: Ditto. This addition has been done since the AppKit 
+        method we use to do the centering, -[NSView scrollRectToVisible:],  does not alter 
+        the view if the rectangle passed to it is already in view. When forceCentering is
+        true, extra math is done to make scrollRectToVisible center the rectangle we want.
+        (-[NSView _KWQ_scrollFrameToVisible]): Pass NO for forceCentering in call through to 
+        _KWQ_scrollRectToVisible:forceCentering:
+        (-[NSView _KWQ_scrollRectToVisible:forceCentering:]): Add forceCentering argument.
+        (-[NSView _KWQ_scrollRectToVisible:inView:forceCentering:]): Ditto.
+        (-[NSClipView _KWQ_scrollRectToVisible:inView:forceCentering:]): Ditto. Do extra
+        math to implement the forceCentering effect.
+        * kwq/KWQScrollView.h: Add forceCentering default argument to ensureRectVisibleCentered.
+        * kwq/KWQScrollView.mm:
+        (QScrollView::ensureRectVisibleCentered): Ditto.
+        * kwq/WebCoreBridge.h:
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge centerSelectionInVisibleArea]): New function. Call through to KWQKHTMLPart. 
+
 2004-09-28  Chris Blumenberg  <cblu@apple.com>
 
        Fixed: WebArchives begin with "<#document/>"
index 45b18d5d498ce852284355efaf33078ed310be57..6107f2f008fed378e37edadcb12398c2d1dac195 100644 (file)
@@ -226,6 +226,7 @@ public:
 
     QRect selectionRect() const;
     NSRect visibleSelectionRect() const;
+    void centerSelectionInVisibleArea() const;
     NSImage *selectionImage() const;
     NSImage *snapshotDragImage(DOM::Node node, NSRect *imageRect, NSRect *elementRect) const;
 
index e0858c83d0a0b07d12c055bc78fa5de3f4c51912..3478dbabb7976589693d316bb877ceb2ada3fc9c 100644 (file)
@@ -3261,6 +3261,25 @@ NSRect KWQKHTMLPart::visibleSelectionRect() const
     return NSIntersectionRect(selectionRect(), [documentView visibleRect]);     
 }
 
+void KWQKHTMLPart::centerSelectionInVisibleArea() const
+{
+    switch (selection().state()) {
+        case Selection::NONE:
+            break;
+        case Selection::CARET: {
+            if (view())
+                // passing true forces centering even if selection is already exposed
+                view()->ensureRectVisibleCentered(selection().caretRect(), true);
+            break;
+        }
+        case Selection::RANGE:
+            if (view())
+                // passing true forces centering even if selection is already exposed
+                view()->ensureRectVisibleCentered(selectionRect(), true);
+            break;
+    }
+}
+
 NSImage *KWQKHTMLPart::imageFromRect(NSRect rect) const
 {
     NSView *view = d->m_view->getDocumentView();
index 7192c21457dfa7a97fb7b901c202f977f46dbab7..2d7f8821c26d98c719c636c47234112d21e276bc 100644 (file)
@@ -28,8 +28,8 @@
 @interface NSView (KWQNSViewExtras)
 
 - (void)_KWQ_scrollFrameToVisible;
-- (void)_KWQ_scrollRectToVisible:(NSRect)rect;
-- (void)_KWQ_scrollRectToVisible:(NSRect)rect inView:(NSView *)view;
+- (void)_KWQ_scrollRectToVisible:(NSRect)rect forceCentering:(BOOL)forceCentering;
+- (void)_KWQ_scrollRectToVisible:(NSRect)rect inView:(NSView *)view forceCentering:(BOOL)forceCentering;
 
 - (void)_KWQ_scrollPointRecursive:(NSPoint)p;
 - (void)_KWQ_scrollPointRecursive:(NSPoint)p inView:(NSView *)view;
index 0bf05ba6209690cbfa90170bfd4c23cf884b693e..503b6ea0a3944200c15180bdf1e1f230d0520b14 100644 (file)
 
 - (void)_KWQ_scrollFrameToVisible
 {
-    [self _KWQ_scrollRectToVisible:[self bounds]];
+    [self _KWQ_scrollRectToVisible:[self bounds] forceCentering:NO];
 }
 
-- (void)_KWQ_scrollRectToVisible:(NSRect)rect
+- (void)_KWQ_scrollRectToVisible:(NSRect)rect forceCentering:(BOOL)forceCentering
 {
-    [self _KWQ_scrollRectToVisible:rect inView:self];
+    [self _KWQ_scrollRectToVisible:rect inView:self forceCentering:forceCentering];
 }
 
-- (void)_KWQ_scrollRectToVisible:(NSRect)rect inView:(NSView *)view
+- (void)_KWQ_scrollRectToVisible:(NSRect)rect inView:(NSView *)view forceCentering:(BOOL)forceCentering
 {
-    [[self superview] _KWQ_scrollRectToVisible:rect inView:view];
+    [[self superview] _KWQ_scrollRectToVisible:rect inView:view forceCentering:forceCentering];
 }
 
 - (void)_KWQ_scrollPointRecursive:(NSPoint)p
 
 @implementation NSClipView (KWQNSViewExtras)
 
-- (void)_KWQ_scrollRectToVisible:(NSRect)rect inView:(NSView *)view
+- (void)_KWQ_scrollRectToVisible:(NSRect)rect inView:(NSView *)view forceCentering:(BOOL)forceCentering
 {
     NSRect exposeRect = [self convertRect:rect fromView:view];
     NSRect visibleRect = [self bounds];
     
-    if (!NSContainsRect(visibleRect, exposeRect)) {
+    if (forceCentering || !NSContainsRect(visibleRect, exposeRect)) {
         // Make an expose rectangle that will end up centering the passed-in rectangle horizontally.
         if (exposeRect.size.width >= visibleRect.size.width) {
+            if (forceCentering) {
+                float expLeft = exposeRect.origin.x;
+                float expRight = exposeRect.origin.x + exposeRect.size.width;
+                float visLeft = visibleRect.origin.x;
+                float visRight = visibleRect.origin.x + visibleRect.size.width;
+                float diffLeft = visLeft - expLeft;
+                float diffRight = expRight - visRight;
+                if (diffLeft >= 0 && diffRight >= 0) {
+                    // Exposed rect fills the visible rect.
+                    // We don't want the view to budge.
+                    exposeRect.origin.x = visLeft;
+                }
+                else if (diffLeft < 0) {
+                    // Scroll so left of visible region shows left of expose rect.
+                    // Leave expose origin as it is to make this happen.
+                }
+                else {
+                    // Scroll so right of visible region shows right of expose rect.
+                    exposeRect.origin.x = expRight - visibleRect.size.width;
+                }
+            }
             exposeRect.size.width = visibleRect.size.width;
-        } else {
+        } 
+        else {
             exposeRect.origin.x -= (visibleRect.size.width - exposeRect.size.width) / 2.0;
             exposeRect.size.width += (visibleRect.size.width - exposeRect.size.width);
         }
         
         // Make an expose rectangle that will end up centering the passed-in rectangle vertically.
         if (exposeRect.size.height >= visibleRect.size.height) {
+            if (forceCentering) {
+                if ([self isFlipped]) {
+                    float expTop = exposeRect.origin.y;
+                    float expBottom = exposeRect.origin.y + exposeRect.size.height;
+                    float visTop = visibleRect.origin.y;
+                    float visBottom = visibleRect.origin.y + visibleRect.size.height;
+                    float diffTop = visTop - expTop;
+                    float diffBottom = expBottom - visBottom;
+                    if (diffTop >= 0 && diffBottom >= 0) {
+                        // Exposed rect fills the visible rect.
+                        // We don't want the view to budge.
+                        exposeRect.origin.y = visTop;
+                    }
+                    else if (diffTop < 0) {
+                        // Scroll so top of visible region shows top of expose rect.
+                        // Leave expose origin as it is to make this happen.
+                    }
+                    else {
+                        // Scroll so bottom of visible region shows bottom of expose rect.
+                        exposeRect.origin.y = expBottom - visibleRect.size.height;
+                    }
+                }
+                else {
+                    float expTop = exposeRect.origin.y + exposeRect.size.height;
+                    float expBottom = exposeRect.origin.y;
+                    float visTop = visibleRect.origin.y + visibleRect.size.height;
+                    float visBottom = visibleRect.origin.y;
+                    float diffTop = expTop - visTop;
+                    float diffBottom = visBottom - expBottom;
+                    if (diffTop >= 0 && diffBottom >= 0) {
+                        // Exposed rect fills the visible rect.
+                        // We don't want the view to budge.
+                        exposeRect.origin.y = visBottom;
+                    }
+                    else if (diffTop < 0) {
+                        // Scroll so top of visible region shows top of expose rect.
+                        exposeRect.origin.y = expTop + visibleRect.size.height;
+                    }
+                    else {
+                        // Scroll so bottom of visible region shows bottom of expose rect.
+                        // Leave expose origin as it is to make this happen.
+                    }
+                } 
+            }
             exposeRect.size.height = visibleRect.size.height;
-        } else {
+        } 
+        else {
             if ([self isFlipped]) {
                 exposeRect.origin.y -= (visibleRect.size.height - exposeRect.size.height) / 2.0;
-            } else {
+            } 
+            else {
                 exposeRect.origin.y += (visibleRect.size.height - exposeRect.size.height) / 2.0;
             }
             exposeRect.size.height += (visibleRect.size.height - exposeRect.size.height);
         [self scrollRectToVisible:exposeRect];
     }
 
-    [super _KWQ_scrollRectToVisible:rect inView:view];
+    [super _KWQ_scrollRectToVisible:rect inView:view forceCentering:forceCentering];
 }
 
 
index 452d02011f3fc7f4b12e067dbb5df4faa29488a8..06ac4f80c4efa2d33cc776b00955910c2db98fce 100644 (file)
@@ -91,7 +91,7 @@ public:
 
     void ensureVisible(int,int);
     void ensureVisible(int,int,int,int);
-    void ensureRectVisibleCentered(const QRect &r);
+    void ensureRectVisibleCentered(const QRect &r, bool forceCentering=false);
         
     NSView *getDocumentView() const;
 };
index f0769040f020461a65d56549cee8544e1b88c2cd..f7632c63b56c6a394280a25f80034566d032d08e 100644 (file)
@@ -477,10 +477,10 @@ void QScrollView::ensureVisible(int x, int y, int xmargin, int ymargin)
     KWQ_UNBLOCK_EXCEPTIONS;
 }
 
-void QScrollView::ensureRectVisibleCentered(const QRect &rect)
+void QScrollView::ensureRectVisibleCentered(const QRect &rect, bool forceCentering)
 {
     KWQ_BLOCK_EXCEPTIONS;
-    [getDocumentView() _KWQ_scrollRectToVisible:NSMakeRect(rect.x(), rect.y(), rect.width(), rect.height())];
+    [getDocumentView() _KWQ_scrollRectToVisible:NSMakeRect(rect.x(), rect.y(), rect.width(), rect.height()) forceCentering:forceCentering];
     KWQ_UNBLOCK_EXCEPTIONS;
 }
 
index 15a776138b049b5c4d1a02b891077ae417170153..4130170fc9abad267408748869749110b749ed49 100644 (file)
@@ -257,6 +257,7 @@ typedef enum {
 
 - (NSRect)selectionRect;
 - (NSRect)visibleSelectionRect;
+- (void)centerSelectionInVisibleArea;
 - (NSImage *)selectionImage;
 - (NSRect)caretRectAtNode:(DOMNode *)node offset:(int)offset;
 
index c2a9e8e49f41c0ca1b9939e8924b83418e140316..08bb376b14ffd88d4851c9974c3e7b0bf3785ce3 100644 (file)
@@ -1182,6 +1182,11 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
     return _part->visibleSelectionRect(); 
 }
 
+- (void)centerSelectionInVisibleArea
+{
+    _part->centerSelectionInVisibleArea(); 
+}
+
 - (NSRect)caretRectAtNode:(DOMNode *)node offset:(int)offset
 {
     return [node _nodeImpl]->renderer()->caretRect(offset, true);
index 87b77b96ae79690908698a38081afe1ba5601121..d3340564512bd980224dc84f7ad2782abc241b0b 100644 (file)
@@ -1,3 +1,16 @@
+2004-09-29  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by Hyatt
+
+        Fix for this bug:
+        
+        <rdar://problem/3818296> REGRESSION (Mail): centerSelectionInVisibleArea does not work correctly
+
+        * WebView.subproj/WebHTMLView.m:
+        (-[WebHTMLView centerSelectionInVisibleArea:]): Now calls new centerSelectionInVisibleArea
+        bridge function instead of ensureCaretVisible. Now handles caret selections and range
+        selections correctly.
+
 2004-09-28  Chris Blumenberg  <cblu@apple.com>
 
        Added timing code so that Doug can time RTF conversion. 
index c61f340478baec195edc6a43c145e852fe956936..a1a7763e6579ed2832a05ba5e1940ec325465d43 100644 (file)
@@ -2697,8 +2697,7 @@ static WebHTMLView *lastHitView = nil;
 
 - (void)centerSelectionInVisibleArea:(id)sender
 {
-    // FIXME: Does this do the right thing when the selection is not a caret?
-    [[self _bridge] ensureCaretVisible];
+    [[self _bridge] centerSelectionInVisibleArea];
 }
 
 - (void)_alterCurrentSelection:(WebSelectionAlteration)alteration direction:(WebSelectionDirection)direction granularity:(WebSelectionGranularity)granularity