Fix case where accessible scrollToMakeVisible didn't work.

Source/core/accessibility/AXObject.cpp

 /*
  * Copyright (C) 2008, 2009, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 643 matching lines @@
 // logic is the same. The goal is to compute the best scroll offset
 // in order to make an object visible within a viewport.
 //
 // In case the whole object cannot fit, you can specify a
 // subfocus - a smaller region within the object that should
 // be prioritized. If the whole object can fit, the subfocus is
 // ignored.
 //
 // Example: the viewport is scrolled to the right just enough
 // that the object is in view.
 //   Before:

[aboxhall, 2014/10/17 19:30:09]

Is this description/ascii art still accurate?

[dmazzoni, 2014/10/17 19:41:35]

Oh - it didn't mention centering. Updated.

 //   +----------Viewport---------+
 //                         +---Object---+
 //                         +--SubFocus--+
 //
 //   After:
 //          +----------Viewport---------+
 //                         +---Object---+
 //                         +--SubFocus--+
 //
 // When constraints cannot be fully satisfied, the min
 // (left/top) position takes precedence over the max (right/bottom).
 //
 // Note that the return value represents the ideal new scroll offset.
 // This may be out of range - the calling function should clip this
 // to the available range.
 static int computeBestScrollOffset(int currentScrollOffset, int subfocusMin, int subfocusMax, int objectMin, int objectMax, int viewportMin, int viewportMax)
 {
     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 reading is that it's not actually
changing the size of the object, but reducing the size of the portion of it to
be scrolled into view. Perhaps something like 'If the ..., consider only the
portion centred on the subfocus which will fit inside the viewport'?

[dmazzoni, 2014/10/17 19:41:35]

Done.

     // direction of subfocus.
     if (objectMax - objectMin > viewportSize) {
         // Subfocus must be within focus:
         subfocusMin = std::max(subfocusMin, objectMin);
         subfocusMax = std::min(subfocusMax, objectMax);
 
         // Subfocus must be no larger than the viewport size; favor top/left.
         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 at the nearest edge as we did
previously?

[dmazzoni, 2014/10/17 19:41:35]

I mentioned this in the change description - does that explanation make sense?

+        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.
     if (objectMin - currentScrollOffset >= viewportMin
         && 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
 {
     // Search up the parent chain until we find the first one that's scrollable.
     AXObject* scrollParent = parentObject();
     ScrollableArea* scrollableArea = 0;
     while (scrollParent) {
         scrollableArea = scrollParent->getScrollableAreaIfScrollable();
         if (scrollableArea)
@@ skipping 13 matching lines @@
         objectRect.x(), objectRect.maxX(),
         0, scrollVisibleRect.width());
     int desiredY = computeBestScrollOffset(
         scrollPosition.y(),
         objectRect.y() + subfocus.y(), objectRect.y() + subfocus.maxY(),
         objectRect.y(), objectRect.maxY(),
         0, scrollVisibleRect.height());
 
     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
 {
     // Search up the parent chain and create a vector of all scrollable parent objects
     // and ending with this object itself.
     Vector<const AXObject*> objects;
     AXObject* parentObject;
     for (parentObject = this->parentObject(); parentObject; parentObject = parentObject->parentObject()) {
         if (parentObject->getScrollableAreaIfScrollable())
@@ skipping 130 matching lines @@
         return ToggleButtonRole;
     if (ariaHasPopup())
         return PopUpButtonRole;
     // We don't contemplate RadioButtonRole, as it depends on the input
     // type.
 
     return ButtonRole;
 }
 
 } // namespace blink