Changeset 58428 in webkit


Ignore:
Timestamp:
Apr 28, 2010 1:57:02 PM (14 years ago)
Author:
Darin Adler
Message:

2010-04-28 Darin Adler <Darin Adler>

Reviewed by Adele Peterson.

REGRESSION: Autoscroll does not work in Mail messages
https://bugs.webkit.org/show_bug.cgi?id=38267
rdar://problem/7559799

The machinery to make autoscrolling work on Mac OS X when a WebView is embedded in another
view had gotten broken in multiple ways. For some reason, a combination of bugs made it
partly work until around r48064. This brings it back.

  • WebCoreSupport/WebChromeClient.mm: (WebChromeClient::scrollRectIntoView): When converting coordinates, use the document view rather than the WebView itself. This logic may not be correct for the case where usesDocumentViews is NO, but that is currently an experimental mode and can be fixed later.

2010-04-28 Darin Adler <Darin Adler>

Reviewed by Adele Peterson.

REGRESSION: Autoscroll does not work in Mail messages
https://bugs.webkit.org/show_bug.cgi?id=38267
rdar://problem/7559799

Still haven't figured out a good way to test this with DumpRenderTree
or with Safari. Testing has to be done with Mail for now.

The machinery to make autoscrolling work on Mac OS X when a WebView is embedded in another
view had gotten broken in multiple ways. For some reason, a combination of bugs made it
partly work until around r48064. This brings it back.

There were three problems:

1) Code in EventHandler decided there was nothing to scroll, so didn't start

the autoscroll timer.

2) The wrong rectangle was passed to Chrome::scrollRectIntoView.
3) The Mac WebKit implementation of ChromeClient::scrollRectIntoView did incorrect

coordinate conversion.

I verified that none of these have any effect on regression tests, or behavior in
web browsers, or behavior on platforms other than Mac.

  • page/EventHandler.cpp: (WebCore::canAutoscroll): Added. Returns true for boxes that can scroll directly and for the top level box of the top frame. (WebCore::EventHandler::handleMouseDraggedEvent): Use canAutoscroll. (WebCore::EventHandler::updateAutoscrollRenderer): Ditto.
  • page/FrameView.cpp: (WebCore::FrameView::scrollToAnchor): Fixed comment.
  • platform/ScrollView.cpp: (WebCore::ScrollView::scrollRectIntoViewRecursively): Put ASSERT_NOT_REACHED into this now-unused function along with some comments about removing some obsolete code.
  • rendering/RenderLayer.cpp: (WebCore::RenderLayer::scrollRectToVisible): Removed call to scrollRectIntoViewRecursively since from the WebKit point of view this is the topmost scroll view anyway. Instead call setScrollPosition. Moved the code to call Chrome::scrollRectIntoView here since it needs to use a different rectangle anyway.
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebCore/ChangeLog

    r58426 r58428  
     12010-04-28  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Adele Peterson.
     4
     5        REGRESSION: Autoscroll does not work in Mail messages
     6        https://bugs.webkit.org/show_bug.cgi?id=38267
     7        rdar://problem/7559799
     8
     9        Still haven't figured out a good way to test this with DumpRenderTree
     10        or with Safari. Testing has to be done with Mail for now.
     11
     12        The machinery to make autoscrolling work on Mac OS X when a WebView is embedded in another
     13        view had gotten broken in multiple ways. For some reason, a combination of bugs made it
     14        partly work until around r48064. This brings it back.
     15
     16        There were three problems:
     17
     18            1) Code in EventHandler decided there was nothing to scroll, so didn't start
     19               the autoscroll timer.
     20            2) The wrong rectangle was passed to Chrome::scrollRectIntoView.
     21            3) The Mac WebKit implementation of ChromeClient::scrollRectIntoView did incorrect
     22               coordinate conversion.
     23
     24        I verified that none of these have any effect on regression tests, or behavior in
     25        web browsers, or behavior on platforms other than Mac.
     26
     27        * page/EventHandler.cpp:
     28        (WebCore::canAutoscroll): Added. Returns true for boxes that can scroll directly
     29        and for the top level box of the top frame.
     30        (WebCore::EventHandler::handleMouseDraggedEvent): Use canAutoscroll.
     31        (WebCore::EventHandler::updateAutoscrollRenderer): Ditto.
     32
     33        * page/FrameView.cpp:
     34        (WebCore::FrameView::scrollToAnchor): Fixed comment.
     35
     36        * platform/ScrollView.cpp:
     37        (WebCore::ScrollView::scrollRectIntoViewRecursively): Put ASSERT_NOT_REACHED into this
     38        now-unused function along with some comments about removing some obsolete code.
     39
     40        * rendering/RenderLayer.cpp:
     41        (WebCore::RenderLayer::scrollRectToVisible): Removed call to scrollRectIntoViewRecursively
     42        since from the WebKit point of view this is the topmost scroll view anyway. Instead call
     43        setScrollPosition. Moved the code to call Chrome::scrollRectIntoView here since it needs
     44        to use a different rectangle anyway.
     45
    1462010-04-21  Ojan Vafai  <ojan@chromium.org>
    247
  • trunk/WebCore/page/EventHandler.cpp

    r58324 r58428  
    457457}
    458458
     459// There are two kinds of renderer that can autoscroll.
     460static bool canAutoscroll(RenderObject* renderer)
     461{
     462    if (!renderer->isBox())
     463        return false;
     464
     465    // Check for a box that can be scrolled in its own right.
     466    if (toRenderBox(renderer)->canBeScrolledAndHasScrollableArea())
     467        return true;
     468
     469    // Check for a box that represents the top level of a web page.
     470    // This can be scrolled by calling Chrome::scrollRectIntoView.
     471    // This only has an effect on the Mac platform in applications
     472    // that put web views into scrolling containers, such as Mac OS X Mail.
     473    // The code for this is in RenderLayer::scrollRectToVisible.
     474    if (renderer->node() != renderer->document())
     475        return false;
     476    Frame* frame = renderer->document()->frame();
     477    if (!frame)
     478        return false;
     479    Page* page = frame->page();
     480    return page && page->mainFrame() == frame;
     481}
     482
    459483#if ENABLE(DRAG_SUPPORT)
    460484bool EventHandler::handleMouseDraggedEvent(const MouseEventWithHitTestResults& event)
     
    477501
    478502    if (m_mouseDownMayStartAutoscroll && !m_panScrollInProgress) {           
    479         // If the selection is contained in a layer that can scroll, that layer should handle the autoscroll
    480         // Otherwise, let the bridge handle it so the view can scroll itself.
     503        // Find a renderer that can autoscroll.
    481504        RenderObject* renderer = targetNode->renderer();
    482         while (renderer && (!renderer->isBox() || !toRenderBox(renderer)->canBeScrolledAndHasScrollableArea())) {
     505        while (renderer && !canAutoscroll(renderer)) {
    483506            if (!renderer->parent() && renderer->node() == renderer->document() && renderer->document()->ownerElement())
    484507                renderer = renderer->document()->ownerElement()->renderer();
     
    783806        m_autoscrollRenderer = nodeAtPoint->renderer();
    784807
    785     while (m_autoscrollRenderer && (!m_autoscrollRenderer->isBox() || !toRenderBox(m_autoscrollRenderer)->canBeScrolledAndHasScrollableArea()))
     808    while (m_autoscrollRenderer && !canAutoscroll(m_autoscrollRenderer))
    786809        m_autoscrollRenderer = m_autoscrollRenderer->parent();
    787810}
  • trunk/WebCore/page/FrameView.cpp

    r58051 r58428  
    14251425        m_frame->document()->axObjectCache()->handleScrolledToAnchor(anchorNode.get());
    14261426
    1427     // scrollRectToVisible can call into scrollRectIntoViewRecursively(), which resets m_maintainScrollPositionAnchor.
     1427    // scrollRectToVisible can call into setScrollPosition(), which resets m_maintainScrollPositionAnchor.
    14281428    m_maintainScrollPositionAnchor = anchorNode;
    14291429}
  • trunk/WebCore/platform/ScrollView.cpp

    r57277 r58428  
    296296}
    297297
    298 void ScrollView::scrollRectIntoViewRecursively(const IntRect& r)
    299 {
    300     // FIXME: This method is not transform-aware.  It should just be moved to FrameView so that an accurate
    301     // position for the child view can be determined.
    302     IntRect rect = r;
    303     ScrollView* view = this;
    304     while (view) {
    305         if (view->prohibitsScrolling()) // Allow the views to scroll into view recursively until we hit one that prohibits scrolling.
    306             return;
    307         view->setScrollPosition(rect.location());
    308         rect.move(view->x() - view->scrollOffset().width(), view->y() - view->scrollOffset().height());
    309         if (view->parent())
    310             rect.intersect(view->frameRect());
    311         view = view->parent();
    312     }
    313    
    314     // We may be embedded inside some containing platform scroll view that we don't manage.  This is the case
    315     // in Mail.app on OS X, for example, where the WebKit view for message bodies is inside a Cocoa NSScrollView
    316     // that contains both it and message headers.  Let the HostWindow know about this scroll so that it can pass the message
    317     // on up the view chain.
    318     // This rect is not clamped, since Mail actually relies on receiving an unclamped rect with negative coordinates in order to
    319     // expose the headers.
    320     if (hostWindow())
    321         hostWindow()->scrollRectIntoView(rect, this);
     298void ScrollView::scrollRectIntoViewRecursively(const IntRect&)
     299{
     300    // FIXME: This function is unused. To clean up further the following can be done:
     301    //
     302    //   1) This function and FrameView::scrollRectIntoViewRecursively
     303    //      can be deleted.
     304    //   2) The FrameView::setScrollPosition can be made non-virtual,
     305    //      since there is no class derived from FrameView. Or the
     306    //      ScrollView::setScrollPosition should be made virtual.
     307    //   3) The scrollRectIntoView function can be removed from the
     308    //      HostWindow class.
     309    //   4) The Chrome::scrollRectIntoView function can be made
     310    //      non-virtual.
     311    //   5) The unused ScrollView* argument can be removed from both
     312    //      Chrome::scrollRectIntoView and ChromeClient::scrollRectIntoView.
     313    //
     314    ASSERT_NOT_REACHED();
    322315}
    323316
  • trunk/WebCore/rendering/RenderLayer.cpp

    r58177 r58428  
    4848#include "CSSStyleDeclaration.h"
    4949#include "CSSStyleSelector.h"
     50#include "Chrome.h"
    5051#include "Document.h"
    5152#include "EventHandler.h"
     
    8081#include "SelectionController.h"
    8182#include "TextStream.h"
     83#include "TransformState.h"
    8284#include "TransformationMatrix.h"
    83 #include "TransformState.h"
    8485#include "TranslateTransformOperation.h"
    85 #include <wtf/text/CString.h>
    8686#include <wtf/StdLibExtras.h>
    8787#include <wtf/UnusedParam.h>
     88#include <wtf/text/CString.h>
    8889
    8990#if USE(ACCELERATED_COMPOSITING)
     
    13251326}
    13261327
    1327 void RenderLayer::scrollRectToVisible(const IntRect &rect, bool scrollToAnchor, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
     1328void RenderLayer::scrollRectToVisible(const IntRect& rect, bool scrollToAnchor, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
    13281329{
    13291330    RenderLayer* parentLayer = 0;
     
    13901391                IntRect r = getRectToExpose(viewRect, rect, alignX, alignY);
    13911392               
    1392                 // If this is the outermost view that RenderLayer needs to scroll, then we should scroll the view recursively
    1393                 // Other apps, like Mail, rely on this feature.
    1394                 frameView->scrollRectIntoViewRecursively(r);
     1393                frameView->setScrollPosition(r.location());
     1394
     1395                // This is the outermost view of a web page, so after scrolling this view we
     1396                // scroll its container by calling Page::scrollRectIntoView.
     1397                // This only has an effect on the Mac platform in applications
     1398                // that put web views into scrolling containers, such as Mac OS X Mail.
     1399                // The canAutoscroll function in EventHandler also knows about this.
     1400                if (Frame* frame = frameView->frame()) {
     1401                    if (Page* page = frame->page())
     1402                        page->chrome()->scrollRectIntoView(rect, 0);
     1403                }
    13951404            }
    13961405        }
  • trunk/WebKit/mac/ChangeLog

    r58386 r58428  
     12010-04-28  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Adele Peterson.
     4
     5        REGRESSION: Autoscroll does not work in Mail messages
     6        https://bugs.webkit.org/show_bug.cgi?id=38267
     7        rdar://problem/7559799
     8
     9        The machinery to make autoscrolling work on Mac OS X when a WebView is embedded in another
     10        view had gotten broken in multiple ways. For some reason, a combination of bugs made it
     11        partly work until around r48064. This brings it back.
     12
     13        * WebCoreSupport/WebChromeClient.mm:
     14        (WebChromeClient::scrollRectIntoView): When converting coordinates, use the document view
     15        rather than the WebView itself. This logic may not be correct for the case where
     16        usesDocumentViews is NO, but that is currently an experimental mode and can be fixed later.
     17
    1182010-04-27  Shinichiro Hamaji  <hamaji@chromium.org>
    219
  • trunk/WebKit/mac/WebCoreSupport/WebChromeClient.mm

    r57903 r58428  
    11/*
    2  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
    33 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
    44 *
     
    5555#import <WebCore/FrameLoadRequest.h>
    5656#import <WebCore/Geolocation.h>
     57#import <WebCore/HTMLNames.h>
    5758#import <WebCore/HitTestResult.h>
    58 #import <WebCore/HTMLNames.h>
    5959#import <WebCore/Icon.h>
    6060#import <WebCore/IntRect.h>
     
    6363#import <WebCore/PlatformString.h>
    6464#import <WebCore/ResourceRequest.h>
    65 #import <WebCore/ScrollView.h>
    6665#import <WebCore/Widget.h>
    6766#import <WebCore/WindowFeatures.h>
     
    500499}
    501500
    502 void WebChromeClient::scrollRectIntoView(const IntRect& r, const ScrollView* scrollView) const
    503 {
    504     // FIXME: This scrolling behavior should be under the control of the embedding client (rather than something
    505     // we just do ourselves).
    506 
     501void WebChromeClient::scrollRectIntoView(const IntRect& r, const ScrollView*) const
     502{
     503    // FIXME: This scrolling behavior should be under the control of the embedding client,
     504    // perhaps in a delegate method, rather than something WebKit does unconditionally.
     505    NSView *coordinateView = [m_webView _usesDocumentViews]
     506        ? [[[m_webView mainFrame] frameView] documentView] : m_webView;
    507507    NSRect rect = r;
    508508    for (NSView *view = m_webView; view; view = [view superview]) {
     
    510510            NSClipView *clipView = (NSClipView *)view;
    511511            NSView *documentView = [clipView documentView];
    512             [documentView scrollRectToVisible:[documentView convertRect:rect fromView:m_webView]];
     512            [documentView scrollRectToVisible:[documentView convertRect:rect fromView:coordinateView]];
    513513        }
    514514    }
Note: See TracChangeset for help on using the changeset viewer.