Attachment 339895 Details for Bug 176615 – patch

[patch] patch

patch.txt (text/plain), 7.24 KB, created by Nan Wang on 2018-05-08 16:22:29 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Nan Wang

Created: 2018-05-08 16:22:29 PDT

Size: 7.24 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 231516)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2018-05-08  Nan Wang  <n_wang@apple.com>
>+
>+        AX: VoiceOver iframe scrolling focus jumping bug
>+        https://bugs.webkit.org/show_bug.cgi?id=176615
>+        <rdar://problem/34333067>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Scrolling to make elements visible is not working correctly for elements inside an
>+        offscreen iframe. The fix contains two parts:
>+        1. For scrolling to make the iframe visible, we should use elementRect() of the scroll view 
>+           instead of boundingBoxRect(). Because the latter method returns an empty rect.
>+        2. We don't need to consider the scroll parent's rect for the sub-focus rect since we
>+           already include that in calling computeBestScrollOffset.
>+
>+        Test: accessibility/scroll-to-make-visible-iframe-offscreen.html
>+
>+        * accessibility/AccessibilityObject.cpp:
>+        (WebCore::AccessibilityObject::scrollToMakeVisibleWithSubFocus const):
>+
> 2018-05-08  Jer Noble  <jer.noble@apple.com>
> 
>         Unreviewed build fix; add missing function definition.
>Index: Source/WebCore/accessibility/AccessibilityObject.cpp
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityObject.cpp	(revision 231385)
>+++ Source/WebCore/accessibility/AccessibilityObject.cpp	(working copy)
>@@ -2997,7 +2997,7 @@ void AccessibilityObject::scrollToMakeVi
>     if (!scrollableArea)
>         return;
> 
>-    LayoutRect objectRect = boundingBoxRect();
>+    LayoutRect objectRect = isScrollView() ? elementRect() : boundingBoxRect();
>     IntPoint scrollPosition = scrollableArea->scrollPosition();
>     // FIXME: unclear if we need LegacyIOSDocumentVisibleRect.
>     IntRect scrollVisibleRect = scrollableArea->visibleContentRect(ScrollableArea::LegacyIOSDocumentVisibleRect);
>@@ -3023,9 +3023,7 @@ void AccessibilityObject::scrollToMakeVi
>     // Convert the subfocus into the coordinates of the scroll parent.
>     IntRect newSubfocus = subfocus;
>     IntRect newElementRect = snappedIntRect(elementRect());
>-    IntRect scrollParentRect = snappedIntRect(scrollParent->elementRect());
>     newSubfocus.move(newElementRect.x(), newElementRect.y());
>-    newSubfocus.move(-scrollParentRect.x(), -scrollParentRect.y());
>     
>     // Recursively make sure the scroll parent itself is visible.
>     if (scrollParent->parentObject())
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 231385)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2018-05-08  Nan Wang  <n_wang@apple.com>
>+
>+        AX: VoiceOver iframe scrolling focus jumping bug
>+        https://bugs.webkit.org/show_bug.cgi?id=176615
>+        <rdar://problem/34333067>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * accessibility/scroll-to-make-visible-iframe-offscreen-expected.txt: Added.
>+        * accessibility/scroll-to-make-visible-iframe-offscreen.html: Added.
>+
> 2018-05-04  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Rebaseline tests for iOS after r231359.
>Index: LayoutTests/accessibility/scroll-to-make-visible-iframe-offscreen-expected.txt
>===================================================================
>--- LayoutTests/accessibility/scroll-to-make-visible-iframe-offscreen-expected.txt	(nonexistent)
>+++ LayoutTests/accessibility/scroll-to-make-visible-iframe-offscreen-expected.txt	(working copy)
>@@ -0,0 +1,18 @@
>+This tests that scrolling to make an element visible works properly when there's an iframe off screen.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+5000-pixel box
>+
>+PASS window.pageYOffset is 0
>+PASS scrolledYOffset > 0 is true
>+PASS frameWindow.pageYOffset >= frameMinYOffset is true
>+PASS frameWindow.pageYOffset <= frameMaxYOffset is true
>+PASS window.pageYOffset == scrolledYOffset is true
>+PASS frameWindow.pageYOffset >= frameMinYOffset is true
>+PASS frameWindow.pageYOffset <= frameMaxYOffset is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>Index: LayoutTests/accessibility/scroll-to-make-visible-iframe-offscreen.html
>===================================================================
>--- LayoutTests/accessibility/scroll-to-make-visible-iframe-offscreen.html	(nonexistent)
>+++ LayoutTests/accessibility/scroll-to-make-visible-iframe-offscreen.html	(working copy)
>@@ -0,0 +1,77 @@
>+<!DOCTYPE html>
>+<head>
>+<script src="../resources/js-test.js"></script>
>+</head>
>+<body>
>+
>+<p id="description"></p>
>+
>+<div style='border: 1px solid #000; height: 5000px;'>5000-pixel box</div>
>+
>+<iframe id="frame" src="data:text/html,<body><button id='upper_target'>Upper Target</button><div style='border: 1px solid #000; height: 5000px;'>5000-pixel box</div><button id='lower_target'>Lower Target</button></body>"></iframe>
>+
>+<div id="console"></div>
>+
>+<script>
>+description("This tests that scrolling to make an element visible works properly when there's an iframe off screen.");
>+
>+window.jsTestIsAsync = true;
>+
>+function runTest() {
>+
>+    window.frame = document.getElementById("frame");
>+    window.frameWindow = frame.contentWindow;
>+    window.frameDoc = frameWindow.document;
>+
>+    var upperTarget = frameDoc.getElementById("upper_target");
>+    var lowerTarget = frameDoc.getElementById("lower_target");
>+
>+    var lowerTargetAccessibleObject;
>+    var upperTargetAccessibleObject;
>+    if (frameWindow.accessibilityController) {
>+        lowerTargetAccessibleObject = frameWindow.accessibilityController.accessibleElementById("lower_target");
>+        upperTargetAccessibleObject = frameWindow.accessibilityController.accessibleElementById("upper_target");
>+    }
>+
>+    // Initial state
>+    window.scrollTo(0, 0);
>+    shouldBeZero("window.pageYOffset");
>+
>+    // Scroll to make lower target visible and check.
>+    if (frameWindow.accessibilityController)
>+        lowerTargetAccessibleObject.scrollToMakeVisible();
>+
>+    // The iframe should be scrolled into view.
>+    window.scrolledYOffset = window.pageYOffset;
>+    shouldBeTrue("scrolledYOffset > 0");
>+
>+    // The content inside the iframe should be scrolled into view too
>+    window.frameMinYOffset = lowerTarget.offsetTop + lowerTarget.offsetHeight - frameWindow.innerHeight;
>+    window.frameMaxYOffset = lowerTarget.offsetTop;
>+    shouldBeTrue("frameWindow.pageYOffset >= frameMinYOffset");
>+    shouldBeTrue("frameWindow.pageYOffset <= frameMaxYOffset");
>+
>+    // Scroll to make upper target visible and check.
>+    if (window.accessibilityController)
>+        upperTargetAccessibleObject.scrollToMakeVisible();
>+
>+    // The iframe should be visible already
>+    shouldBeTrue("window.pageYOffset == scrolledYOffset");
>+
>+    // The content inside the iframe should be scrolled into view too
>+    window.frameMinYOffset = upperTarget.offsetTop + upperTarget.offsetHeight - frameWindow.innerHeight;
>+    window.frameMaxYOffset = upperTarget.offsetTop;
>+    shouldBeTrue("frameWindow.pageYOffset >= frameMinYOffset");
>+    shouldBeTrue("frameWindow.pageYOffset <= frameMaxYOffset");
>+
>+    finishJSTest();
>+}
>+
>+window.addEventListener("load", function() {
>+    setTimeout(runTest, 0);
>+}, false);
>+
>+</script>
>+
>+</body>
>+</html>

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 176615: 320276 | 339895 | 339911 | 339916 | 339925 | 340008