Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(323)

Unified Diff: Source/core/accessibility/AXObject.cpp

Issue 661183002: Fix case where accessible scrollToMakeVisible didn't work. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « LayoutTests/accessibility/scroll-to-make-visible-nested-2-expected.txt ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/accessibility/AXObject.cpp
diff --git a/Source/core/accessibility/AXObject.cpp b/Source/core/accessibility/AXObject.cpp
index c296ddfbda8c8dcc53211a281e49abb716bcb372..b916975bb761f0f71cde29c3d51a83a8106ab80e 100644
--- a/Source/core/accessibility/AXObject.cpp
+++ b/Source/core/accessibility/AXObject.cpp
@@ -681,7 +681,7 @@ static int computeBestScrollOffset(int currentScrollOffset, int subfocusMin, int
{
int viewportSize = viewportMax - viewportMin;
- // If the focus size is larger than the viewport size, shrink it in the
+ // If the object size is larger than the viewport size, shrink it in the
aboxhall 2014/10/17 19:30:09 I think 'shrink it' is a little confusing - my rea
dmazzoni 2014/10/17 19:41:35 Done.
// direction of subfocus.
if (objectMax - objectMin > viewportSize) {
// Subfocus must be within focus:
@@ -692,12 +692,12 @@ static int computeBestScrollOffset(int currentScrollOffset, int subfocusMin, int
if (subfocusMax - subfocusMin > viewportSize)
subfocusMax = subfocusMin + viewportSize;
- if (subfocusMin + viewportSize > objectMax) {
- objectMin = objectMax - viewportSize;
- } else {
- objectMin = subfocusMin;
- objectMax = subfocusMin + viewportSize;
- }
+ // Compute the size of an object centered on the subfocus, the size of the viewport.
aboxhall 2014/10/17 19:30:09 Why do we want to centre it rather than keeping it
dmazzoni 2014/10/17 19:41:35 I mentioned this in the change description - does
+ int centeredObjectMin = (subfocusMin + subfocusMax - viewportSize) / 2;
+ int centeredObjectMax = centeredObjectMin + viewportSize;
+
+ objectMin = std::max(objectMin, centeredObjectMin);
+ objectMax = std::min(objectMax, centeredObjectMax);
}
// Exit now if the focus is already within the viewport.
@@ -705,18 +705,8 @@ static int computeBestScrollOffset(int currentScrollOffset, int subfocusMin, int
&& objectMax - currentScrollOffset <= viewportMax)
return currentScrollOffset;
- // Scroll left if we're too far to the right.
- if (objectMax - currentScrollOffset > viewportMax)
- return objectMax - viewportMax;
-
- // Scroll right if we're too far to the left.
- if (objectMin - currentScrollOffset < viewportMin)
- return objectMin - viewportMin;
-
- ASSERT_NOT_REACHED();
-
- // This shouldn't happen.
- return currentScrollOffset;
+ // Center the object in the viewport.
+ return (objectMin + objectMax - viewportMin - viewportMax) / 2;
}
void AXObject::scrollToMakeVisibleWithSubFocus(const IntRect& subfocus) const
@@ -750,9 +740,16 @@ void AXObject::scrollToMakeVisibleWithSubFocus(const IntRect& subfocus) const
scrollParent->scrollTo(IntPoint(desiredX, desiredY));
+ // Convert the subfocus into the coordinates of the scroll parent.
+ IntRect newSubfocus = subfocus;
+ IntRect newElementRect = pixelSnappedIntRect(elementRect());
+ IntRect scrollParentRect = pixelSnappedIntRect(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())
- scrollParent->scrollToMakeVisible();
+ scrollParent->scrollToMakeVisibleWithSubFocus(newSubfocus);
}
void AXObject::scrollToGlobalPoint(const IntPoint& globalPoint) const
« no previous file with comments | « LayoutTests/accessibility/scroll-to-make-visible-nested-2-expected.txt ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698