Diff
Modified: trunk/Source/WebCore/ChangeLog (270608 => 270609)
--- trunk/Source/WebCore/ChangeLog 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/ChangeLog 2020-12-10 00:35:27 UTC (rev 270609)
@@ -1,3 +1,44 @@
+2020-12-09 Andres Gonzalez <[email protected]>
+
+ Fix for focus tracking in isolated tree mode.
+ https://bugs.webkit.org/show_bug.cgi?id=219662
+
+ Reviewed by Chris Fleizach.
+
+ Covered by existing tests.
+
+ - AXIsolatedTree::setFocusedNodeID and applyPendingChanges now properly
+ handle the focused node ID update when the focused object changes.
+ - AccessibilityObject::setFocused sets focus and activates the
+ corresponding view. This was done in the wrapper baseAccessibilitySetFocus
+ method, but this is a more appropriate place for this core functionality.
+ - Some code cleanup, ASSERT checks of appropriate thread, and additional
+ logging.
+
+ * accessibility/AccessibilityObject.cpp:
+ (WebCore::AccessibilityObject::setFocused):
+ * accessibility/AccessibilityObject.h:
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::setFocused):
+ * accessibility/AccessibilityScrollView.cpp:
+ (WebCore::AccessibilityScrollView::setFocused):
+ * accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
+ (-[WebAccessibilityObjectWrapper _accessibilitySetFocus:]):
+ * accessibility/isolatedtree/AXIsolatedObject.cpp:
+ (WebCore::AXIsolatedObject::page const):
+ (WebCore::AXIsolatedObject::document const):
+ (WebCore::AXIsolatedObject::documentFrameView const):
+ * accessibility/isolatedtree/AXIsolatedTree.cpp:
+ (WebCore::AXIsolatedTree::setFocusedNodeID):
+ (WebCore::AXIsolatedTree::applyPendingChanges):
+ * accessibility/mac/WebAccessibilityObjectWrapperBase.h:
+ * accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
+ (-[WebAccessibilityObjectWrapperBase baseAccessibilitySetFocus:]):
+ Deleted, not needed since core functionality is now in AccessibilityObject::setFocused.
+ * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+ (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
+ (-[WebAccessibilityObjectWrapper _accessibilitySetValue:forAttribute:]):
+
2020-12-09 Said Abou-Hallawa <[email protected]>
[GPU Process]: Recording an in-process ImageBuffer drawing has to convert it to a NativeImage first
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (270608 => 270609)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2020-12-10 00:35:27 UTC (rev 270609)
@@ -2561,7 +2561,40 @@
auto* axObjectCache = this->axObjectCache();
return page && axObjectCache ? axObjectCache->focusedObjectForPage(page) : nullptr;
}
-
+
+void AccessibilityObject::setFocused(bool focus)
+{
+ if (focus) {
+ // Ensure that the view is focused and active, otherwise, any attempt to set focus to an object inside it will fail.
+ auto* document = this->document();
+ if (!document)
+ return;
+
+ auto* frame = document->frame();
+ if (frame && frame->selection().isFocusedAndActive())
+ return; // Nothing to do, already focused and active.
+
+ auto* page = document->page();
+ if (!page)
+ return;
+
+ ChromeClient& chromeClient = page->chrome().client();
+ chromeClient.focus();
+
+#if PLATFORM(COCOA)
+ auto* frameView = documentFrameView();
+ if (!frameView)
+ return;
+
+ // Legacy WebKit1 case.
+ if (frameView->platformWidget())
+ chromeClient.makeFirstResponder((NSResponder *)frameView->platformWidget());
+ else
+ chromeClient.assistiveTechnologyMakeFirstResponder();
+#endif
+ }
+}
+
AccessibilitySortDirection AccessibilityObject::sortDirection() const
{
AccessibilityRole role = roleValue();
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (270608 => 270609)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.h 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h 2020-12-10 00:35:27 UTC (rev 270609)
@@ -457,7 +457,10 @@
unsigned hierarchicalLevel() const override { return 0; }
bool isInlineText() const override;
- void setFocused(bool) override { }
+ // Ensures that the view is focused and active before attempting to set focus to an AccessibilityObject.
+ // Subclasses that override setFocused should call this base implementation first.
+ void setFocused(bool) override;
+
void setSelectedText(const String&) override { }
void setSelectedTextRange(const PlainTextRange&) override { }
bool setValue(const String&) override { return false; }
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (270608 => 270609)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2020-12-10 00:35:27 UTC (rev 270609)
@@ -1849,7 +1849,7 @@
{
if (!canSetFocusAttribute())
return;
-
+
Document* document = this->document();
Node* node = this->node();
@@ -1858,11 +1858,14 @@
return;
}
+ // Call the base class setFocused to ensure the view is focused and active.
+ AccessibilityObject::setFocused(on);
+
// When a node is told to set focus, that can cause it to be deallocated, which means that doing
// anything else inside this object will crash. To fix this, we added a RefPtr to protect this object
// long enough for duration.
RefPtr<AccessibilityObject> protectedThis(this);
-
+
// If this node is already the currently focused node, then calling focus() won't do anything.
// That is a problem when focus is removed from the webpage to chrome, and then returns.
// In these cases, we need to do what keyboard and mouse focus do, which is reset focus first.
Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp (270608 => 270609)
--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp 2020-12-10 00:35:27 UTC (rev 270609)
@@ -104,9 +104,12 @@
AccessibilityObject* webArea = webAreaObject();
return webArea && webArea->isFocused();
}
-
+
void AccessibilityScrollView::setFocused(bool focused)
{
+ // Call the base class setFocused to ensure the view is focused and active.
+ AccessibilityObject::setFocused(focused);
+
if (AccessibilityObject* webArea = webAreaObject())
webArea->setFocused(focused);
}
Modified: trunk/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm (270608 => 270609)
--- trunk/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm 2020-12-10 00:35:27 UTC (rev 270609)
@@ -2081,7 +2081,8 @@
- (void)_accessibilitySetFocus:(BOOL)focus
{
- [self baseAccessibilitySetFocus:focus];
+ if (auto* backingObject = self.axBackingObject)
+ backingObject->setFocused(focus);
}
- (void)accessibilityDecreaseSelection:(TextGranularity)granularity
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp (270608 => 270609)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp 2020-12-10 00:35:27 UTC (rev 270609)
@@ -1905,8 +1905,11 @@
Page* AXIsolatedObject::page() const
{
- if (auto* object = associatedAXObject())
- return object->page();
+ ASSERT(isMainThread());
+
+ if (auto* axObject = associatedAXObject())
+ return axObject->page();
+
ASSERT_NOT_REACHED();
return nullptr;
}
@@ -1913,8 +1916,11 @@
Document* AXIsolatedObject::document() const
{
- if (auto* object = associatedAXObject())
- return object->document();
+ ASSERT(isMainThread());
+
+ if (auto* axObject = associatedAXObject())
+ return axObject->document();
+
ASSERT_NOT_REACHED();
return nullptr;
}
@@ -1921,8 +1927,12 @@
FrameView* AXIsolatedObject::documentFrameView() const
{
- if (auto* object = associatedAXObject())
- return object->documentFrameView();
+ ASSERT(isMainThread());
+
+ if (auto* axObject = associatedAXObject())
+ return axObject->documentFrameView();
+
+ ASSERT_NOT_REACHED();
return nullptr;
}
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp (270608 => 270609)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp 2020-12-10 00:35:27 UTC (rev 270609)
@@ -388,8 +388,13 @@
AXTRACE("AXIsolatedTree::setFocusedNodeID");
AXLOG(makeString("axID ", axID));
ASSERT(isMainThread());
+
LockHolder locker { m_changeLogLock };
m_pendingFocusedNodeID = axID;
+
+ AXPropertyMap propertyMap;
+ propertyMap.set(AXPropertyName::IsFocused, true);
+ m_pendingPropertyChanges.append({ axID, propertyMap });
}
void AXIsolatedTree::removeNode(AXID axID)
@@ -436,9 +441,18 @@
LockHolder locker { m_changeLogLock };
- AXLOG(makeString("focusedNodeID ", m_focusedNodeID, " pendingFocusedNodeID ", m_pendingFocusedNodeID));
- m_focusedNodeID = m_pendingFocusedNodeID;
+ if (m_pendingFocusedNodeID != m_focusedNodeID) {
+ AXLOG(makeString("focusedNodeID ", m_focusedNodeID, " pendingFocusedNodeID ", m_pendingFocusedNodeID));
+ if (m_focusedNodeID != InvalidAXID) {
+ // Set the old focused object's IsFocused property to false.
+ AXPropertyMap propertyMap;
+ propertyMap.set(AXPropertyName::IsFocused, false);
+ m_pendingPropertyChanges.append({ m_focusedNodeID, propertyMap });
+ }
+ m_focusedNodeID = m_pendingFocusedNodeID;
+ }
+
while (m_pendingNodeRemovals.size()) {
auto axID = m_pendingNodeRemovals.takeLast();
AXLOG(makeString("removing axID ", axID));
Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h (270608 => 270609)
--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h 2020-12-10 00:35:27 UTC (rev 270609)
@@ -74,7 +74,6 @@
- (NSString *)baseAccessibilityHelpText;
- (NSArray<NSString *> *)baseAccessibilitySpeechHint;
-- (void)baseAccessibilitySetFocus:(BOOL)focus;
- (NSString *)ariaLandmarkRoleDescription;
- (id)attachmentView;
Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm (270608 => 270609)
--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm 2020-12-10 00:35:27 UTC (rev 270609)
@@ -43,8 +43,6 @@
#import "AccessibilityTableCell.h"
#import "AccessibilityTableColumn.h"
#import "AccessibilityTableRow.h"
-#import "Chrome.h"
-#import "ChromeClient.h"
#import "ColorMac.h"
#import "ContextMenuController.h"
#import "Editing.h"
@@ -469,27 +467,6 @@
return self.axBackingObject->ariaLandmarkRoleDescription();
}
-- (void)baseAccessibilitySetFocus:(BOOL)focus
-{
- // If focus is just set without making the view the first responder, then keyboard focus won't move to the right place.
- if (focus && !self.axBackingObject->document()->frame()->selection().isFocusedAndActive()) {
- FrameView* frameView = self.axBackingObject->documentFrameView();
- Page* page = self.axBackingObject->page();
- if (page && frameView) {
- ChromeClient& chromeClient = page->chrome().client();
- chromeClient.focus();
-
- // Legacy WebKit1 case.
- if (frameView->platformWidget())
- chromeClient.makeFirstResponder(frameView->platformWidget());
- else
- chromeClient.assistiveTechnologyMakeFirstResponder();
- }
- }
-
- self.axBackingObject->setFocused(focus);
-}
-
- (NSString *)accessibilityPlatformMathSubscriptKey
{
ASSERT_NOT_REACHED();
Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm (270608 => 270609)
--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm 2020-12-10 00:23:43 UTC (rev 270608)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm 2020-12-10 00:35:27 UTC (rev 270609)
@@ -2139,9 +2139,12 @@
ALLOW_DEPRECATED_IMPLEMENTATIONS_END
{
AXTRACE(makeString("WebAccessibilityObjectWrapper accessibilityAttributeValue:", String(attributeName)));
+
auto* backingObject = self.updateObjectBackingStore;
- if (!backingObject)
+ if (!backingObject) {
+ AXLOG("No backingObject!!!");
return nil;
+ }
if (backingObject->isDetachedFromParent()) {
AXLOG("backingObject is detached from parent!!!");
@@ -3360,11 +3363,15 @@
#endif
}
-- (void)_accessibilitySetValue:(id)value forAttribute:(NSString*)attributeName
+- (void)_accessibilitySetValue:(id)value forAttribute:(NSString *)attributeName
{
+ AXTRACE(makeString("WebAccessibilityObjectWrapper _accessibilitySetValue: forAttribute:", String(attributeName)));
+
auto* backingObject = self.updateObjectBackingStore;
- if (!backingObject)
+ if (!backingObject) {
+ AXLOG("No backingObject!!!");
return;
+ }
AXTextMarkerRangeRef textMarkerRange = nil;
NSNumber* number = nil;
@@ -3392,7 +3399,7 @@
backingObject->setSelectedVisiblePositionRange(visiblePositionRangeForTextMarkerRange(backingObject->axObjectCache(), textMarkerRange));
});
} else if ([attributeName isEqualToString: NSAccessibilityFocusedAttribute]) {
- [self baseAccessibilitySetFocus:[number boolValue]];
+ backingObject->setFocused([number boolValue]);
} else if ([attributeName isEqualToString: NSAccessibilityValueAttribute]) {
if (number && backingObject->canSetNumericValue())
backingObject->setValue([number floatValue]);
