Attachment 281675 Details for Bug 158629 – Patch

[patch] Patch

bug-158629-20160620140440.patch (text/plain), 34.89 KB, created by Simon Fraser (smfr) on 2016-06-20 14:04:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2016-06-20 14:04:47 PDT

Size: 34.89 KB

patch

obsolete

>Subversion Revision: 202170
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8ea2308fb8132da49bd9012263669e2e24c4cdad..33c648d1be5b56eaa7b77477ff9c5db8054584c5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,57 @@
>+2016-06-20  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Focus event dispatched in iframe causes parent document to scroll incorrectly
>+        https://bugs.webkit.org/show_bug.cgi?id=158629
>+        rdar://problem/26521616
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When focussing elements in iframes, the page could scroll to an incorrect location.
>+        This happened because code in Element::focus() tried to disable scrolling on focus,
>+        but did so only for the current frame, so ancestor frames got programmatically scrolled.
>+        On iOS we handle the scrolling in the UI process, so never want the web process to
>+        do programmatic scrolling.
>+
>+        Fix by changing the focus and cache restore code to use SelectionRevealMode::DoNotReveal,
>+        rather than manually prohibiting frame scrolling. Pass SelectionRevealMode through various callers,
>+        and use RevealUpToMainFrame for iOS, allowing the UI process to do the zoomToRect: for the main frame.
>+
>+        Tests: fast/forms/ios/focus-input-in-iframe.html
>+               fast/forms/ios/programmatic-focus-input-in-iframe.html
>+
>+        * dom/Document.h:
>+        * dom/Element.cpp:
>+        (WebCore::Element::scrollIntoView):
>+        (WebCore::Element::scrollIntoViewIfNeeded):
>+        (WebCore::Element::scrollIntoViewIfNotVisible):
>+        (WebCore::Element::focus):
>+        (WebCore::Element::updateFocusAppearance):
>+        * dom/Element.h:
>+        * editing/Editor.cpp:
>+        (WebCore::Editor::insertTextWithoutSendingTextEvent):
>+        (WebCore::Editor::revealSelectionAfterEditingOperation):
>+        (WebCore::Editor::findStringAndScrollToVisible):
>+        * editing/FrameSelection.cpp:
>+        (WebCore::FrameSelection::updateAndRevealSelection):
>+        (WebCore::FrameSelection::revealSelection):
>+        (WebCore::FrameSelection::FrameSelection): Deleted.
>+        * editing/FrameSelection.h:
>+        * html/HTMLInputElement.cpp:
>+        (WebCore::HTMLInputElement::updateFocusAppearance):
>+        * html/HTMLTextAreaElement.cpp:
>+        (WebCore::HTMLTextAreaElement::updateFocusAppearance):
>+        * page/ContextMenuController.cpp:
>+        (WebCore::ContextMenuController::contextMenuItemSelected):
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::scrollToAnchor):
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::scrollRectToVisible):
>+        (WebCore::RenderLayer::autoscroll):
>+        * rendering/RenderLayer.h:
>+        * rendering/RenderObject.cpp:
>+        (WebCore::RenderObject::scrollRectToVisible):
>+        * rendering/RenderObject.h:
>+
> 2016-06-17  Per Arne Vollan  <pvollan@apple.com>
> 
>         [Win] Scrolling in popup menu scrolls past last entry.
>diff --git a/Source/WebKit/mac/ChangeLog b/Source/WebKit/mac/ChangeLog
>index 757d2a0423576d8413482a75b35e08dc699f2e52..0c2c57a1ba2db8ec3f8253567f99e5f799ed1135 100644
>--- a/Source/WebKit/mac/ChangeLog
>+++ b/Source/WebKit/mac/ChangeLog
>@@ -1,3 +1,21 @@
>+2016-06-20  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Focus event dispatched in iframe causes parent document to scroll incorrectly
>+        https://bugs.webkit.org/show_bug.cgi?id=158629
>+        rdar://problem/26521616
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Pass SelectionRevealMode::Reveal in existing code.
>+
>+        * WebView/WebFrame.mm:
>+        (-[WebFrame _scrollDOMRangeToVisible:]):
>+        (-[WebFrame _scrollDOMRangeToVisible:withInset:]):
>+        (-[WebFrame revealSelectionAtExtent:]):
>+        * WebView/WebHTMLView.mm:
>+        (-[WebHTMLView jumpToSelection:]):
>+        (-[WebHTMLView centerSelectionInVisibleArea:]):
>+
> 2016-06-15  Alexey Proskuryakov  <ap@apple.com>
> 
>         +[WebHTMLRepresentation supportedMIMETypes] leaks
>diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
>index 902c2a9d434bb7ab18731a2455975eec18047cc8..6cacddf21c00bcc8ebb54a1174c54e8a0391c017 100644
>--- a/Source/WebKit2/ChangeLog
>+++ b/Source/WebKit2/ChangeLog
>@@ -1,3 +1,16 @@
>+2016-06-20  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Focus event dispatched in iframe causes parent document to scroll incorrectly
>+        https://bugs.webkit.org/show_bug.cgi?id=158629
>+        rdar://problem/26521616
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Pass SelectionRevealMode::Reveal in existing code.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::centerSelectionInVisibleArea):
>+
> 2016-06-16  Chris Dumez  <cdumez@apple.com>
> 
>         No need to ref connection in lambda inside NetworkResourceLoader::tryStoreAsCacheEntry()
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 0b839618f5ba299164e16e8a8036d7c2a1867b12..5017c822f4ed3ce0e6d68eedbf12a7f3dbe10d03 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -282,11 +282,6 @@ enum class SelectionRestorationMode {
>     SetDefault,
> };
> 
>-enum class SelectionRevealMode {
>-    Reveal,
>-    DoNotReveal
>-};
>-
> enum class HttpEquivPolicy {
>     Enabled,
>     DisabledBySettings,
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index eebb715b0970467e71344bb7b8ba52abd054b6b5..ce7c2cd81623ec18e5ecfb00f69417500dcef7f4 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -639,9 +639,9 @@ void Element::scrollIntoView(bool alignToTop)
>     LayoutRect bounds = renderer()->anchorRect();
>     // Align to the top / bottom and to the closest edge.
>     if (alignToTop)
>-        renderer()->scrollRectToVisible(bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
>+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
>     else
>-        renderer()->scrollRectToVisible(bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignBottomAlways);
>+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignBottomAlways);
> }
> 
> void Element::scrollIntoViewIfNeeded(bool centerIfNeeded)
>@@ -653,9 +653,9 @@ void Element::scrollIntoViewIfNeeded(bool centerIfNeeded)
> 
>     LayoutRect bounds = renderer()->anchorRect();
>     if (centerIfNeeded)
>-        renderer()->scrollRectToVisible(bounds, ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded);
>+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded);
>     else
>-        renderer()->scrollRectToVisible(bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
> }
> 
> void Element::scrollIntoViewIfNotVisible(bool centerIfNotVisible)
>@@ -667,9 +667,9 @@ void Element::scrollIntoViewIfNotVisible(bool centerIfNotVisible)
>     
>     LayoutRect bounds = renderer()->anchorRect();
>     if (centerIfNotVisible)
>-        renderer()->scrollRectToVisible(bounds, ScrollAlignment::alignCenterIfNotVisible, ScrollAlignment::alignCenterIfNotVisible);
>+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignCenterIfNotVisible, ScrollAlignment::alignCenterIfNotVisible);
>     else
>-        renderer()->scrollRectToVisible(bounds, ScrollAlignment::alignToEdgeIfNotVisible, ScrollAlignment::alignToEdgeIfNotVisible);
>+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNotVisible, ScrollAlignment::alignToEdgeIfNotVisible);
> }
>     
> void Element::scrollByUnits(int units, ScrollGranularity granularity)
>@@ -2245,20 +2245,18 @@ void Element::focus(bool restorePreviousSelection, FocusDirection direction)
>     }
>         
>     cancelFocusAppearanceUpdate();
>+
>+    SelectionRevealMode revealMode = SelectionRevealMode::Reveal;
> #if PLATFORM(IOS)
>     // Focusing a form element triggers animation in UIKit to scroll to the right position.
>     // Calling updateFocusAppearance() would generate an unnecessary call to ScrollView::setScrollPosition(),
>     // which would jump us around during this animation. See <rdar://problem/6699741>.
>-    FrameView* view = document().view();
>-    bool isFormControl = view && is<HTMLFormControlElement>(*this);
>-    if (isFormControl)
>-        view->setProhibitsScrolling(true);
>-#endif
>-    updateFocusAppearance(restorePreviousSelection ? SelectionRestorationMode::Restore : SelectionRestorationMode::SetDefault);
>-#if PLATFORM(IOS)
>+    bool isFormControl = is<HTMLFormControlElement>(*this);
>     if (isFormControl)
>-        view->setProhibitsScrolling(false);
>+        revealMode = SelectionRevealMode::RevealUpToMainFrame;
> #endif
>+
>+    updateFocusAppearance(restorePreviousSelection ? SelectionRestorationMode::Restore : SelectionRestorationMode::SetDefault, revealMode);
> }
> 
> void Element::updateFocusAppearanceAfterAttachIfNeeded()
>@@ -2290,11 +2288,10 @@ void Element::updateFocusAppearance(SelectionRestorationMode, SelectionRevealMod
>         
>         if (frame->selection().shouldChangeSelection(newSelection)) {
>             frame->selection().setSelection(newSelection, FrameSelection::defaultSetSelectionOptions(), Element::defaultFocusTextStateChangeIntent());
>-            if (revealMode == SelectionRevealMode::Reveal)
>-                frame->selection().revealSelection();
>+            frame->selection().revealSelection(revealMode);
>         }
>-    } else if (renderer() && !renderer()->isWidget() && revealMode == SelectionRevealMode::Reveal)
>-        renderer()->scrollRectToVisible(renderer()->anchorRect());
>+    } else if (renderer() && !renderer()->isWidget())
>+        renderer()->scrollRectToVisible(revealMode, renderer()->anchorRect());
> }
> 
> void Element::blur()
>diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h
>index 2faa64b319856d777458a800d9219fdbb97b620f..8ca64d4eedb32a4ce5f54af21d84d0318c568a99 100644
>--- a/Source/WebCore/dom/Element.h
>+++ b/Source/WebCore/dom/Element.h
>@@ -60,6 +60,12 @@ enum SpellcheckAttributeState {
>     SpellcheckAttributeDefault
> };
> 
>+enum class SelectionRevealMode {
>+    Reveal,
>+    RevealUpToMainFrame, // Scroll overflow and iframes, but not the main frame.
>+    DoNotReveal
>+};
>+
> class Element : public ContainerNode {
> public:
>     static Ref<Element> create(const QualifiedName&, Document&);
>diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
>index 34cdce99f3bff32d469f451697d4d59da708c5fa..b74597aa39baada34fa62dc859de94b65e6dac69 100644
>--- a/Source/WebCore/editing/Editor.cpp
>+++ b/Source/WebCore/editing/Editor.cpp
>@@ -1204,7 +1204,7 @@ bool Editor::insertTextWithoutSendingTextEvent(const String& text, bool selectIn
>             // Reveal the current selection
>             if (Frame* editedFrame = document->frame())
>                 if (Page* page = editedFrame->page())
>-                    page->focusController().focusedOrMainFrame().selection().revealSelection(ScrollAlignment::alignCenterIfNeeded);
>+                    page->focusController().focusedOrMainFrame().selection().revealSelection(SelectionRevealMode::Reveal, ScrollAlignment::alignCenterIfNeeded);
>         }
>     }
> 
>@@ -2799,7 +2799,7 @@ void Editor::revealSelectionAfterEditingOperation(const ScrollAlignment& alignme
>     if (m_ignoreCompositionSelectionChange)
>         return;
> 
>-    m_frame.selection().revealSelection(alignment, revealExtentOption);
>+    m_frame.selection().revealSelection(SelectionRevealMode::Reveal, alignment, revealExtentOption);
> }
> 
> void Editor::setIgnoreCompositionSelectionChange(bool ignore, RevealSelection shouldRevealExistingSelection)
>@@ -3147,7 +3147,7 @@ RefPtr<Range> Editor::findStringAndScrollToVisible(const String& target, Range*
>     if (!nextMatch)
>         return nullptr;
> 
>-    nextMatch->firstNode()->renderer()->scrollRectToVisible(nextMatch->absoluteBoundingBox(),
>+    nextMatch->firstNode()->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, nextMatch->absoluteBoundingBox(),
>         ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded);
> 
>     return nextMatch;
>diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
>index a56e6b7d56126794071d0fbb59df1b49ff7f0d49..33834ad096e81822516e7813b2b80dc9bb26e2d4 100644
>--- a/Source/WebCore/editing/FrameSelection.cpp
>+++ b/Source/WebCore/editing/FrameSelection.cpp
>@@ -127,7 +127,6 @@ FrameSelection::FrameSelection(Frame* frame)
> #if PLATFORM(IOS)
>     , m_updateAppearanceEnabled(false)
>     , m_caretBlinks(true)
>-    , m_scrollingSuppressCount(0)
> #endif
> {
>     if (shouldAlwaysUseDirectionalSelection(m_frame))
>@@ -388,7 +387,7 @@ void FrameSelection::updateAndRevealSelection(const AXTextStateChangeIntent& int
>         else
>             alignment = m_alwaysAlignCursorOnScrollWhenRevealingSelection ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignToEdgeIfNeeded;
> 
>-        revealSelection(alignment, RevealExtent);
>+        revealSelection(SelectionRevealMode::Reveal, alignment, RevealExtent);
>     }
> 
>     notifyAccessibilityForSelectionChange(intent);
>@@ -2290,8 +2289,11 @@ HTMLFormElement* FrameSelection::currentForm() const
>     return scanForForm(start);
> }
> 
>-void FrameSelection::revealSelection(const ScrollAlignment& alignment, RevealExtentOption revealExtentOption)
>+void FrameSelection::revealSelection(SelectionRevealMode revealMode, const ScrollAlignment& alignment, RevealExtentOption revealExtentOption)
> {
>+    if (revealMode == SelectionRevealMode::DoNotReveal)
>+        return;
>+
>     LayoutRect rect;
> 
>     switch (m_selection.selectionType()) {
>@@ -2312,7 +2314,7 @@ void FrameSelection::revealSelection(const ScrollAlignment& alignment, RevealExt
>         if (RenderLayer* layer = start.deprecatedNode()->renderer()->enclosingLayer()) {
>             if (!m_scrollingSuppressCount) {
>                 layer->setAdjustForIOSCaretWhenScrolling(true);
>-                layer->scrollRectToVisible(rect, alignment, alignment);
>+                layer->scrollRectToVisible(revealMode, rect, alignment, alignment);
>                 layer->setAdjustForIOSCaretWhenScrolling(false);
>                 updateAppearance();
>                 if (m_frame->page())
>@@ -2323,7 +2325,7 @@ void FrameSelection::revealSelection(const ScrollAlignment& alignment, RevealExt
>         // FIXME: This code only handles scrolling the startContainer's layer, but
>         // the selection rect could intersect more than just that.
>         // See <rdar://problem/4799899>.
>-        if (start.deprecatedNode()->renderer()->scrollRectToVisible(rect, alignment, alignment))
>+        if (start.deprecatedNode()->renderer()->scrollRectToVisible(revealMode, rect, alignment, alignment))
>             updateAppearance();
> #endif
>     }
>diff --git a/Source/WebCore/editing/FrameSelection.h b/Source/WebCore/editing/FrameSelection.h
>index 95ac3774b802450db9d8ffed979dd238d05f5e1d..f78f8725263626cc52ad14003be1c4f14bab8c08 100644
>--- a/Source/WebCore/editing/FrameSelection.h
>+++ b/Source/WebCore/editing/FrameSelection.h
>@@ -28,6 +28,7 @@
> 
> #include "AXTextStateChangeIntent.h"
> #include "EditingStyle.h"
>+#include "Element.h"
> #include "IntRect.h"
> #include "LayoutRect.h"
> #include "Range.h"
>@@ -266,7 +267,7 @@ public:
> 
>     WEBCORE_EXPORT HTMLFormElement* currentForm() const;
> 
>-    WEBCORE_EXPORT void revealSelection(const ScrollAlignment& = ScrollAlignment::alignCenterIfNeeded, RevealExtentOption = DoNotRevealExtent);
>+    WEBCORE_EXPORT void revealSelection(SelectionRevealMode = SelectionRevealMode::Reveal, const ScrollAlignment& = ScrollAlignment::alignCenterIfNeeded, RevealExtentOption = DoNotRevealExtent);
>     WEBCORE_EXPORT void setSelectionFromNone();
> 
>     bool shouldShowBlockCursor() const { return m_shouldShowBlockCursor; }
>@@ -350,7 +351,7 @@ private:
>     bool m_updateAppearanceEnabled : 1;
>     bool m_caretBlinks : 1;
>     Color m_caretColor;
>-    int m_scrollingSuppressCount;
>+    int m_scrollingSuppressCount { 0 };
> #endif
> };
> 
>diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp
>index c86ef4a971fec906f432bdcda50b5237f26ae6b6..9631106fdbb5aa5d499fefc46809560eea935620 100644
>--- a/Source/WebCore/html/HTMLInputElement.cpp
>+++ b/Source/WebCore/html/HTMLInputElement.cpp
>@@ -404,8 +404,8 @@ void HTMLInputElement::updateFocusAppearance(SelectionRestorationMode restoratio
>             select(Element::defaultFocusTextStateChangeIntent());
>         else
>             restoreCachedSelection();
>-        if (document().frame() && revealMode == SelectionRevealMode::Reveal)
>-            document().frame()->selection().revealSelection();
>+        if (document().frame())
>+            document().frame()->selection().revealSelection(revealMode);
>     } else
>         HTMLTextFormControlElement::updateFocusAppearance(restorationMode, revealMode);
> }
>diff --git a/Source/WebCore/html/HTMLTextAreaElement.cpp b/Source/WebCore/html/HTMLTextAreaElement.cpp
>index 255bed67972842de343b2e660b13e3b55c6bb97d..f92108aca6f1d8a3d9cedacdb0b8082ddf382c8d 100644
>--- a/Source/WebCore/html/HTMLTextAreaElement.cpp
>+++ b/Source/WebCore/html/HTMLTextAreaElement.cpp
>@@ -260,8 +260,8 @@ void HTMLTextAreaElement::updateFocusAppearance(SelectionRestorationMode restora
>     } else
>         restoreCachedSelection(Element::defaultFocusTextStateChangeIntent());
> 
>-    if (document().frame() && revealMode == SelectionRevealMode::Reveal)
>-        document().frame()->selection().revealSelection();
>+    if (document().frame())
>+        document().frame()->selection().revealSelection(revealMode);
> }
> 
> void HTMLTextAreaElement::defaultEventHandler(Event* event)
>diff --git a/Source/WebCore/page/ContextMenuController.cpp b/Source/WebCore/page/ContextMenuController.cpp
>index ef22e526bbbbb4c548b94a01fe389337763384a6..394bb2c519a67157ceed8aeea6f58f1c78f28146 100644
>--- a/Source/WebCore/page/ContextMenuController.cpp
>+++ b/Source/WebCore/page/ContextMenuController.cpp
>@@ -372,7 +372,7 @@ void ContextMenuController::contextMenuItemSelected(ContextMenuAction action, co
>             ASSERT(document);
>             RefPtr<ReplaceSelectionCommand> command = ReplaceSelectionCommand::create(*document, createFragmentFromMarkup(*document, title, ""), replaceOptions);
>             applyCommand(command);
>-            frame->selection().revealSelection(ScrollAlignment::alignToEdgeIfNeeded);
>+            frame->selection().revealSelection(SelectionRevealMode::Reveal, ScrollAlignment::alignToEdgeIfNeeded);
>         }
>         break;
>     }
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 47e4d5501795a6d678050b45512e6f1d5442cfea..680baa55e560668076bc0cde4eb617e2ee364040 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -3050,11 +3050,11 @@ void FrameView::scrollToAnchor()
>     // Scroll nested layers and frames to reveal the anchor.
>     // Align to the top and to the closest side (this matches other browsers).
>     if (anchorNode->renderer()->style().isHorizontalWritingMode())
>-        anchorNode->renderer()->scrollRectToVisible(rect, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
>+        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
>     else if (anchorNode->renderer()->style().isFlippedBlocksWritingMode())
>-        anchorNode->renderer()->scrollRectToVisible(rect, ScrollAlignment::alignRightAlways, ScrollAlignment::alignToEdgeIfNeeded);
>+        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, ScrollAlignment::alignRightAlways, ScrollAlignment::alignToEdgeIfNeeded);
>     else
>-        anchorNode->renderer()->scrollRectToVisible(rect, ScrollAlignment::alignLeftAlways, ScrollAlignment::alignToEdgeIfNeeded);
>+        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, ScrollAlignment::alignLeftAlways, ScrollAlignment::alignToEdgeIfNeeded);
> 
>     if (AXObjectCache* cache = frame().document()->existingAXObjectCache())
>         cache->handleScrolledToAnchor(anchorNode.get());
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 6a6f116498d1895362a078a8f9b0516d1e310fb0..d02904bd40e533cc768eed5833a25abc8a0aea84 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -80,6 +80,7 @@
> #include "HitTestRequest.h"
> #include "HitTestResult.h"
> #include "Logging.h"
>+#include "MainFrame.h"
> #include "NoEventDispatchAssertion.h"
> #include "OverflowEvent.h"
> #include "OverlapTestRequestClient.h"
>@@ -2490,7 +2491,7 @@ bool RenderLayer::allowsCurrentScroll() const
>     return box->hasHorizontalOverflow() || box->hasVerticalOverflow();
> }
> 
>-void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
>+void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
> {
>     LOG_WITH_STREAM(Scrolling, stream << "Layer " << this << " scrollRectToVisible " << rect);
> 
>@@ -2550,6 +2551,9 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
>                     parentLayer = nullptr;
>             }
>         } else {
>+            if (revealMode == SelectionRevealMode::RevealUpToMainFrame && frameView.frame().isMainFrame())
>+                return;
>+
> #if !PLATFORM(IOS)
>             LayoutRect viewRect = frameView.visibleContentRect();
>             LayoutRect visibleRectRelativeToDocument = viewRect;
>@@ -2574,7 +2578,7 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
>     }
>     
>     if (parentLayer)
>-        parentLayer->scrollRectToVisible(newRect, alignX, alignY);
>+        parentLayer->scrollRectToVisible(revealMode, newRect, alignX, alignY);
> }
> 
> void RenderLayer::updateCompositingLayersAfterScroll()
>@@ -2667,7 +2671,7 @@ LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, const Lay
> void RenderLayer::autoscroll(const IntPoint& position)
> {
>     IntPoint currentDocumentPosition = renderer().view().frameView().windowToContents(position);
>-    scrollRectToVisible(LayoutRect(currentDocumentPosition, LayoutSize(1, 1)), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>+    scrollRectToVisible(SelectionRevealMode::Reveal, LayoutRect(currentDocumentPosition, LayoutSize(1, 1)), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
> }
> 
> bool RenderLayer::canResize() const
>diff --git a/Source/WebCore/rendering/RenderLayer.h b/Source/WebCore/rendering/RenderLayer.h
>index ce190bf174abd751ada7a94fb4d2a5e7b6862403..ee932929121b9b956c2b7f5c8501e0e805339c62 100644
>--- a/Source/WebCore/rendering/RenderLayer.h
>+++ b/Source/WebCore/rendering/RenderLayer.h
>@@ -205,7 +205,7 @@ public:
> 
>     void availableContentSizeChanged(AvailableSizeChangeReason) override;
> 
>-    void scrollRectToVisible(const LayoutRect&, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
>+    void scrollRectToVisible(SelectionRevealMode, const LayoutRect&, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
> 
>     LayoutRect getRectToExpose(const LayoutRect& visibleRect, const LayoutRect& visibleRectRelativeToDocument, const LayoutRect& exposeRect, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
> 
>diff --git a/Source/WebCore/rendering/RenderObject.cpp b/Source/WebCore/rendering/RenderObject.cpp
>index cbd2e64b5b3ef2b44f12cfe58e171d093d132dcb..1b9cbb8711a27edd1025174c1552ff60f21e4926 100644
>--- a/Source/WebCore/rendering/RenderObject.cpp
>+++ b/Source/WebCore/rendering/RenderObject.cpp
>@@ -358,13 +358,16 @@ RenderLayer* RenderObject::enclosingLayer() const
>     return nullptr;
> }
> 
>-bool RenderObject::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
>+bool RenderObject::scrollRectToVisible(SelectionRevealMode revealMode, const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
> {
>+    if (revealMode == SelectionRevealMode::DoNotReveal)
>+        return false;
>+
>     RenderLayer* enclosingLayer = this->enclosingLayer();
>     if (!enclosingLayer)
>         return false;
> 
>-    enclosingLayer->scrollRectToVisible(rect, alignX, alignY);
>+    enclosingLayer->scrollRectToVisible(revealMode, rect, alignX, alignY);
>     return true;
> }
> 
>diff --git a/Source/WebCore/rendering/RenderObject.h b/Source/WebCore/rendering/RenderObject.h
>index e3a7d7f7aee180244d6dbc926ef417c75d27fb0f..40fe1113121aa9594e44998383f0dd2d973b1dd5 100644
>--- a/Source/WebCore/rendering/RenderObject.h
>+++ b/Source/WebCore/rendering/RenderObject.h
>@@ -155,7 +155,7 @@ public:
>     WEBCORE_EXPORT RenderLayer* enclosingLayer() const;
> 
>     // Scrolling is a RenderBox concept, however some code just cares about recursively scrolling our enclosing ScrollableArea(s).
>-    WEBCORE_EXPORT bool scrollRectToVisible(const LayoutRect&, const ScrollAlignment& alignX = ScrollAlignment::alignCenterIfNeeded, const ScrollAlignment& alignY = ScrollAlignment::alignCenterIfNeeded);
>+    WEBCORE_EXPORT bool scrollRectToVisible(SelectionRevealMode, const LayoutRect&, const ScrollAlignment& alignX = ScrollAlignment::alignCenterIfNeeded, const ScrollAlignment& alignY = ScrollAlignment::alignCenterIfNeeded);
> 
>     // Convenience function for getting to the nearest enclosing box of a RenderObject.
>     RenderBox& enclosingBox() const;
>diff --git a/Source/WebKit/mac/WebView/WebFrame.mm b/Source/WebKit/mac/WebView/WebFrame.mm
>index b0dcd9a9aa6aeff4cdf6adde04ef7cf29ae41876..080d6ec1515f5b04f8a33c52e0ec70d75d2efa50 100644
>--- a/Source/WebKit/mac/WebView/WebFrame.mm
>+++ b/Source/WebKit/mac/WebView/WebFrame.mm
>@@ -717,12 +717,12 @@ - (void)_scrollDOMRangeToVisible:(DOMRange *)range
>         
>     if (startNode && startNode->renderer()) {
> #if !PLATFORM(IOS)
>-        startNode->renderer()->scrollRectToVisible(enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>+        startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
> #else
>         RenderLayer* layer = startNode->renderer()->enclosingLayer();
>         if (layer) {
>             layer->setAdjustForIOSCaretWhenScrolling(true);
>-            startNode->renderer()->scrollRectToVisible(enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>+            startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>             layer->setAdjustForIOSCaretWhenScrolling(false);
>             _private->coreFrame->selection().setCaretRectNeedsUpdate();
>             _private->coreFrame->selection().updateAppearance();
>@@ -741,7 +741,7 @@ - (void)_scrollDOMRangeToVisible:(DOMRange *)range withInset:(CGFloat)inset
>         RenderLayer* layer = startNode->renderer()->enclosingLayer();
>         if (layer) {
>             layer->setAdjustForIOSCaretWhenScrolling(true);
>-            startNode->renderer()->scrollRectToVisible(enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>+            startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
>             layer->setAdjustForIOSCaretWhenScrolling(false);
> 
>             Frame *coreFrame = core(self);
>@@ -1375,7 +1375,7 @@ - (void)revealSelectionAtExtent:(BOOL)revealExtent
> {
>     WebCore::Frame *frame = core(self);
>     RevealExtentOption revealExtentOption = revealExtent ? RevealExtent : DoNotRevealExtent;
>-    frame->selection().revealSelection(ScrollAlignment::alignToEdgeIfNeeded, revealExtentOption);
>+    frame->selection().revealSelection(SelectionRevealMode::Reveal, ScrollAlignment::alignToEdgeIfNeeded, revealExtentOption);
> }
> 
> - (void)resetSelection
>diff --git a/Source/WebKit/mac/WebView/WebHTMLView.mm b/Source/WebKit/mac/WebView/WebHTMLView.mm
>index 7ef1a30a9a0b5a337441ddeaa76db7be7fbb75ab..ff41bdda4b88297a499a83dffe2c686197a0545e 100644
>--- a/Source/WebKit/mac/WebView/WebHTMLView.mm
>+++ b/Source/WebKit/mac/WebView/WebHTMLView.mm
>@@ -3146,7 +3146,7 @@ - (void)jumpToSelection:(id)sender
>     COMMAND_PROLOGUE
> 
>     if (Frame* coreFrame = core([self _frame]))
>-        coreFrame->selection().revealSelection(ScrollAlignment::alignCenterAlways);
>+        coreFrame->selection().revealSelection(SelectionRevealMode::Reveal, ScrollAlignment::alignCenterAlways);
> }
> 
> #if !PLATFORM(IOS)
>@@ -5345,7 +5345,7 @@ - (void)centerSelectionInVisibleArea:(id)sender
>     COMMAND_PROLOGUE
> 
>     if (Frame* coreFrame = core([self _frame]))
>-        coreFrame->selection().revealSelection(ScrollAlignment::alignCenterAlways);
>+        coreFrame->selection().revealSelection(SelectionRevealMode::Reveal, ScrollAlignment::alignCenterAlways);
> }
> 
> #if !PLATFORM(IOS)
>diff --git a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
>index 625287ee75b212b48c56fb82bd0e8f7a0b7e1f32..d99546c914c86778885a1edb3168ae61e71e342a 100644
>--- a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
>@@ -2395,7 +2395,7 @@ bool WebPage::scrollBy(uint32_t scrollDirection, uint32_t scrollGranularity)
> void WebPage::centerSelectionInVisibleArea()
> {
>     Frame& frame = m_page->focusController().focusedOrMainFrame();
>-    frame.selection().revealSelection(ScrollAlignment::alignCenterAlways);
>+    frame.selection().revealSelection(SelectionRevealMode::Reveal, ScrollAlignment::alignCenterAlways);
>     m_findController.showFindIndicatorInSelection();
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 017e448488a27be0685fa1fe793073ebe508bc3c..8a30a62d8108d706265a5b1e0dc0e31752f8bd38 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2016-06-20  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Focus event dispatched in iframe causes parent document to scroll incorrectly
>+        https://bugs.webkit.org/show_bug.cgi?id=158629
>+        rdar://problem/26521616
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/forms/ios/focus-input-in-iframe-expected.txt: Added.
>+        * fast/forms/ios/focus-input-in-iframe.html: Added.
>+        * fast/forms/ios/programmatic-focus-input-in-iframe-expected.txt: Added.
>+        * fast/forms/ios/programmatic-focus-input-in-iframe.html: Added.
>+
> 2016-06-17  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Skipping two new LayoutTests that rely on mouse events on ios-simulator.
>diff --git a/LayoutTests/fast/forms/ios/focus-input-in-iframe-expected.txt b/LayoutTests/fast/forms/ios/focus-input-in-iframe-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..f5a2ebd2ffab7139942e2cd76ee7b63b20c76fce
>--- /dev/null
>+++ b/LayoutTests/fast/forms/ios/focus-input-in-iframe-expected.txt
>@@ -0,0 +1,7 @@
>+Tests zooming into a text input on tap.
>+
>+Click to focus input
>+
>+tap location	{ x: 20.000, y: 62.000 }
>+scale	1.455
>+visibleRect	{ left: 0.000, top: 1201.976, width: 219.979, height: 329.968 }
>diff --git a/LayoutTests/fast/forms/ios/focus-input-in-iframe.html b/LayoutTests/fast/forms/ios/focus-input-in-iframe.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c3712def0ae85ff531b20b7f416d5b853f398a54
>--- /dev/null
>+++ b/LayoutTests/fast/forms/ios/focus-input-in-iframe.html
>@@ -0,0 +1,48 @@
>+<!DOCTYPE html> 
>+
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=0.5">
>+    <style>
>+        button {
>+            display: block;
>+        }
>+        iframe {
>+            margin-top: 800px;
>+        }
>+    </style>
>+    
>+    <script src="resources/zooming-test-utils.js"></script>
>+    <script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+
>+    if (window.internals)
>+        internals.settings.setFrameFlatteningEnabled(true);
>+    
>+    function buttonClicked()
>+    {
>+        document.getElementById('frame').contentDocument.getElementById('input').focus();
>+    }
>+
>+    function doTest()
>+    {
>+        testZoomAfterTap(document.getElementById('target'), 10, 10);
>+    }
>+
>+    window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+
>+<p>Tests zooming into a text input on tap.</p>
>+<button id="target" onclick="buttonClicked()">Click to focus input</button>
>+
>+<iframe id="frame" srcdoc="<style>input { margin: 400px 20px; }</style>
>+<input id='input' type='text'>
>+"></iframe>
>+
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/forms/ios/programmatic-focus-input-in-iframe-expected.txt b/LayoutTests/fast/forms/ios/programmatic-focus-input-in-iframe-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..4eef465861ef0702486f7dde50a0de298f18c203
>--- /dev/null
>+++ b/LayoutTests/fast/forms/ios/programmatic-focus-input-in-iframe-expected.txt
>@@ -0,0 +1,4 @@
>+Tests that a programmatic focus should not scroll into view
>+
>+PASS: page did not scroll.
>+
>diff --git a/LayoutTests/fast/forms/ios/programmatic-focus-input-in-iframe.html b/LayoutTests/fast/forms/ios/programmatic-focus-input-in-iframe.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..6edb5fa453924eb0d3385051cfdbaaad484c013c
>--- /dev/null
>+++ b/LayoutTests/fast/forms/ios/programmatic-focus-input-in-iframe.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html> 
>+
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=0.5">
>+    <style>
>+        button {
>+            display: block;
>+        }
>+        iframe {
>+            margin-top: 800px;
>+        }
>+    </style>
>+    
>+    <script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+    
>+    if (window.internals)
>+        internals.settings.setFrameFlatteningEnabled(true);
>+    
>+    function pageDidScroll()
>+    {
>+        document.getElementById('result').textContent = 'FAIL: page scrolled to ' + document.scrollingElement.scrollTop;
>+    }
>+
>+    function doTest()
>+    {
>+        window.setTimeout(function() {
>+            document.getElementById('frame').contentDocument.getElementById('input').focus();
>+        }, 0);
>+
>+        // Wait for any scroll to happen.
>+        window.setTimeout(function() {
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        }, 100);
>+    }
>+
>+    window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body onscroll="pageDidScroll()">
>+
>+<p>Tests that a programmatic focus should not scroll into view</p>
>+<div id="result">
>+    PASS: page did not scroll.
>+</div>
>+
>+<iframe id="frame" srcdoc="<style>input { margin: 400px 20px; }</style>
>+<input id='input' type='text'>
>+"></iframe>
>+
>+</body>
>+</html>

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 158629: 281025 | 281498 | 281675 | 341898