Diff
Modified: trunk/LayoutTests/ChangeLog (228375 => 228376)
--- trunk/LayoutTests/ChangeLog 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/LayoutTests/ChangeLog 2018-02-12 17:16:17 UTC (rev 228376)
@@ -1,3 +1,17 @@
+2018-02-12 Chris Fleizach <cfleiz...@apple.com>
+
+ AX: defer focusedUIElement notifications
+ https://bugs.webkit.org/show_bug.cgi?id=182643
+ <rdar://problem/37394310>
+
+ Reviewed by Zalan Bujtas.
+
+ * accessibility/mac/aria-menu-item-selected-notification.html:
+ Rewrite test to accomodate that focus changes happen asynchronously.
+ * accessibility/mac/selection-notification-focus-change-expected.txt:
+ * platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt:
+ The order of notifications is different now that focus changes happen later.
+
2018-02-12 Per Arne Vollan <pvol...@apple.com>
Update test expectations for some tests which are failing on only one ews Windows bot.
Modified: trunk/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html (228375 => 228376)
--- trunk/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html 2018-02-12 17:16:17 UTC (rev 228376)
@@ -46,14 +46,20 @@
// Trigger notification through focus.
document.getElementById("item1").focus();
- // Trigger notification through aria-selected.
- document.getElementById("item2").setAttribute("aria-selected", "true");
+ setTimeout(function() {
+ // Trigger notification through aria-selected.
+ document.getElementById("item2").setAttribute("aria-selected", "true");
- // Ensure we don't get a notification when aria-selected is false.
- document.getElementById("item2").setAttribute("aria-selected", "false");
+ setTimeout(function() {
+ // Ensure we don't get a notification when aria-selected is false.
+ document.getElementById("item2").setAttribute("aria-selected", "false");
- // Trigger another notification through focus to ensure we don't
- document.getElementById("item3").focus();
+ setTimeout(function() {
+ // Trigger another notification through focus to ensure we don't
+ document.getElementById("item3").focus();
+ }, 1);
+ }, 1);
+ }, 1);
}
</script>
Modified: trunk/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt (228375 => 228376)
--- trunk/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt 2018-02-12 17:16:17 UTC (rev 228376)
@@ -16,9 +16,9 @@
Received AXFocusChanged
eventSender.keyDown(tabCharacter)
-Received AXFocusChanged
Received AXSelectedTextChanged
PASS userInfo["AXTextSelectionChangedFocus"] is true
+Received AXFocusChanged
Received AXSelectedTextChanged
PASS userInfo["AXTextSelectionChangedFocus"] is true
PASS successfullyParsed is true
Modified: trunk/Source/WebCore/ChangeLog (228375 => 228376)
--- trunk/Source/WebCore/ChangeLog 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/Source/WebCore/ChangeLog 2018-02-12 17:16:17 UTC (rev 228376)
@@ -1,3 +1,31 @@
+2018-02-12 Chris Fleizach <cfleiz...@apple.com>
+
+ AX: defer focusedUIElement notifications
+ https://bugs.webkit.org/show_bug.cgi?id=182643
+ <rdar://problem/37394310>
+
+ Reviewed by Zalan Bujtas.
+
+ Deferring focus changes for accessibility has a number of benefits.
+ 1) Reduces the chance of calling into layout during layout.
+ 2) Coalesces multiple focus notifications that would be needlessly sent.
+ 3) Improves performance by not calling out to the accessibility notification machinery during layout.
+
+ In this patch, I also started making more AXObjectCache calls private. This will reduce the chance that clients
+ will call into AXObjectCache during unexpected times.
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::deferFocusedUIElementChangeIfNeeded):
+ (WebCore::conditionallyAddNodeToFilterList):
+ (WebCore::filterVectorPairForRemoval):
+ (WebCore::filterMapForRemoval):
+ (WebCore::filterListForRemoval):
+ (WebCore::AXObjectCache::prepareForDocumentDestruction):
+ (WebCore::AXObjectCache::performDeferredCacheUpdate):
+ * accessibility/AXObjectCache.h:
+ * dom/Document.cpp:
+ (WebCore::Document::setFocusedElement):
+
2018-02-11 Gustavo Noronha Silva <gustavo.noro...@collabora.co.uk>
[GTK] Scrolling sometimes jumps around
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (228375 => 228376)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2018-02-12 17:16:17 UTC (rev 228376)
@@ -1017,6 +1017,14 @@
postNotification(getOrCreate(node), &document(), AXMenuListItemSelected);
}
+void AXObjectCache::deferFocusedUIElementChangeIfNeeded(Node* oldNode, Node* newNode)
+{
+ if (nodeAndRendererAreValid(newNode) && rendererNeedsDeferredUpdate(*newNode->renderer()))
+ m_deferredFocusedNodeChange.append({ oldNode, newNode });
+ else
+ handleFocusedUIElementChanged(oldNode, newNode);
+}
+
void AXObjectCache::handleFocusedUIElementChanged(Node* oldNode, Node* newNode)
{
handleMenuItemSelected(newNode);
@@ -2777,25 +2785,33 @@
return result;
}
-template<typename T, typename U>
-static void filterMapForRemoval(const HashMap<T, U>& list, const Document& document, HashSet<Node*>& nodesToRemove)
+static void conditionallyAddNodeToFilterList(Node* node, const Document& document, HashSet<Node*>& nodesToRemove)
{
+ if (!node->isConnected() || &node->document() == &document)
+ nodesToRemove.add(node);
+}
+
+template<typename T>
+static void filterVectorPairForRemoval(const Vector<std::pair<T, T>>& list, const Document& document, HashSet<Node*>& nodesToRemove)
+{
for (auto& entry : list) {
- auto* node = entry.key;
- if (node->isConnected() && &node->document() != &document)
- continue;
- nodesToRemove.add(node);
+ conditionallyAddNodeToFilterList(entry.first, document, nodesToRemove);
+ conditionallyAddNodeToFilterList(entry.second, document, nodesToRemove);
}
}
+
+template<typename T, typename U>
+static void filterMapForRemoval(const HashMap<T, U>& list, const Document& document, HashSet<Node*>& nodesToRemove)
+{
+ for (auto& entry : list)
+ conditionallyAddNodeToFilterList(entry.key, document, nodesToRemove);
+}
template<typename T>
static void filterListForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove)
{
- for (auto* node : list) {
- if (node->isConnected() && &node->document() != &document)
- continue;
- nodesToRemove.add(node);
- }
+ for (auto* node : list)
+ conditionallyAddNodeToFilterList(node, document, nodesToRemove);
}
void AXObjectCache::prepareForDocumentDestruction(const Document& document)
@@ -2808,6 +2824,7 @@
filterListForRemoval(m_deferredSelectedChildredChangedList, document, nodesToRemove);
filterMapForRemoval(m_deferredTextFormControlValue, document, nodesToRemove);
filterMapForRemoval(m_deferredAttributeChange, document, nodesToRemove);
+ filterVectorPairForRemoval(m_deferredFocusedNodeChange, document, nodesToRemove);
for (auto* node : nodesToRemove)
remove(*node);
@@ -2851,6 +2868,10 @@
for (auto& deferredAttributeChangeContext : m_deferredAttributeChange)
handleAttributeChange(deferredAttributeChangeContext.value, deferredAttributeChangeContext.key);
m_deferredAttributeChange.clear();
+
+ for (auto& deferredFocusedChangeContext : m_deferredFocusedNodeChange)
+ handleFocusedUIElementChanged(deferredFocusedChangeContext.first, deferredFocusedChangeContext.second);
+ m_deferredFocusedNodeChange.clear();
}
void AXObjectCache::deferRecomputeIsIgnoredIfNeeded(Element* element)
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (228375 => 228376)
--- trunk/Source/WebCore/accessibility/AXObjectCache.h 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h 2018-02-12 17:16:17 UTC (rev 228376)
@@ -168,21 +168,13 @@
void childrenChanged(RenderObject*, RenderObject* newChild = nullptr);
void childrenChanged(AccessibilityObject*);
void checkedStateChanged(Node*);
- void selectedChildrenChanged(Node*);
- void selectedChildrenChanged(RenderObject*);
- // Called by a node when text or a text equivalent (e.g. alt) attribute is changed.
- void textChanged(Node*);
// Called when a node has just been attached, so we can make sure we have the right subclass of AccessibilityObject.
void updateCacheAfterNodeIsAttached(Node*);
- void handleActiveDescendantChanged(Node*);
- void handleAriaRoleChanged(Node*);
- void handleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
+ void deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
void handleScrolledToAnchor(const Node* anchorNode);
- void handleAriaExpandedChange(Node*);
void handleScrollbarUpdate(ScrollView*);
- void handleModalChange(Node*);
Node* modalNode();
void deferAttributeChangeIfNeeded(const QualifiedName&, Element*);
@@ -411,11 +403,20 @@
void handleMenuItemSelected(Node*);
void handleAttributeChange(const QualifiedName&, Element*);
bool shouldProcessAttributeChange(const QualifiedName&, Element*);
-
+ void selectedChildrenChanged(Node*);
+ void selectedChildrenChanged(RenderObject*);
+ // Called by a node when text or a text equivalent (e.g. alt) attribute is changed.
+ void textChanged(Node*);
+ void handleActiveDescendantChanged(Node*);
+ void handleAriaRoleChanged(Node*);
+ void handleAriaExpandedChange(Node*);
+ void handleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
+
// aria-modal related
void findModalNodes();
void updateCurrentModalNode();
bool isNodeVisible(Node*) const;
+ void handleModalChange(Node*);
Document& m_document;
HashMap<AXID, RefPtr<AccessibilityObject>> m_objects;
@@ -449,6 +450,7 @@
ListHashSet<Element*> m_deferredSelectedChildredChangedList;
HashMap<Element*, String> m_deferredTextFormControlValue;
HashMap<Element*, QualifiedName> m_deferredAttributeChange;
+ Vector<std::pair<Node*, Node*>> m_deferredFocusedNodeChange;
bool m_isSynchronizingSelection { false };
bool m_performingDeferredCacheUpdate { false };
};
Modified: trunk/Source/WebCore/dom/Document.cpp (228375 => 228376)
--- trunk/Source/WebCore/dom/Document.cpp 2018-02-12 17:13:18 UTC (rev 228375)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-02-12 17:16:17 UTC (rev 228376)
@@ -3965,7 +3965,7 @@
if (!focusChangeBlocked && m_focusedElement) {
// Create the AXObject cache in a focus change because GTK relies on it.
if (AXObjectCache* cache = axObjectCache())
- cache->handleFocusedUIElementChanged(oldFocusedElement.get(), newFocusedElement.get());
+ cache->deferFocusedUIElementChangeIfNeeded(oldFocusedElement.get(), newFocusedElement.get());
}
if (!focusChangeBlocked && page())