<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:cfleizach@apple.com" title="chris fleizach <cfleizach@apple.com>"> <span class="fn">chris fleizach</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: AccessibilityNodeObject::childrenChanged() generates too many AXLiveRegionChanged notifications"
href="https://bugs.webkit.org/show_bug.cgi?id=147211">bug 147211</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #257322 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: AccessibilityNodeObject::childrenChanged() generates too many AXLiveRegionChanged notifications"
href="https://bugs.webkit.org/show_bug.cgi?id=147211#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: AccessibilityNodeObject::childrenChanged() generates too many AXLiveRegionChanged notifications"
href="https://bugs.webkit.org/show_bug.cgi?id=147211">bug 147211</a>
from <span class="vcard"><a class="email" href="mailto:cfleizach@apple.com" title="chris fleizach <cfleizach@apple.com>"> <span class="fn">chris fleizach</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=257322&action=diff" name="attach_257322" title="patch">attachment 257322</a> <a href="attachment.cgi?id=257322&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=257322&action=review">https://bugs.webkit.org/attachment.cgi?id=257322&action=review</a>
<span class="quote">> Source/WebCore/ChangeLog:9
> + AccessibilityNodeObject::childrenChanged() being called many times in a short period of time may cause</span >
change to
AccessibilityNodeObject::childrenChanged() can be called repeatedly, generating a live region change notification each time.
Sometimes, so many happen that VoiceOver hangs. We can use a timer to make sure that we coalesce these notifications.
12
<span class="quote">> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:106
> + , liveRegionNotificationPostTimer(*this, &AccessibilityNodeObject::liveRegionChangedNotificationPostTimerFired)</span >
i think this think should be on AXObjectCache. Right now this timer is being created for every AXObject, which also means that if you stop one then another object
will just as easily fire its own object. we want to have one instance of this timer per axOBjectCache
<span class="quote">> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:162
> + if (liveRegionNotificationPostTimer.isActive())</span >
if the timer is active, we can probably do the opposite and just skip this code, since we now the active timer will now cover this case too?
<span class="quote">> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:166
> + liveRegionNotificationPostTimer.startOneShot(AccessibilityLiveRegionChangeNotificationInterval);</span >
will this work just as well if we just use 0 as the time? i have a feeling that a lot of these are being generated within the same call stack so a 0 second timer may work just as well
<span class="quote">> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:183
> + for (AccessibilityObject* object : liveRegionObjectsSet)</span >
this can be auto& object : liverRegion..
<span class="quote">> Source/WebCore/accessibility/AccessibilityNodeObject.h:210
> + Timer liveRegionNotificationPostTimer;</span >
internal ivars should all start with m_*
<span class="quote">> Source/WebCore/accessibility/AccessibilityNodeObject.h:211
> + HashSet<AccessibilityObject*> liveRegionObjectsSet;</span >
this should be HashSet<RefPtr<AccessibilityObject>>
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-liveregions-notifications.html:48
> + setTimeout("operation4()", 40);</span >
if we make the timeout to be 0 seconds this may not be necessary
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-multiple-liveregions-notification.html:21
> + description("This tests that ARIA live regions are sending out the correct notifications. We perform two operations to each aria region, at the end it should trigger 2 live region notifications");</span >
2 -> two
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-multiple-liveregions-notification.html:31
> + if (liveRegionChangeCount == 2) {</span >
this doesn't necessarily catch if we only have two notifications.
we might have a 100 being fired but after 2 we cleanup.
i think in this case we should have a timeout to finish the test AFTER we get two notifications. It should be short obviously but
that will allow us to check if more notifications have come in
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-multiple-liveregions-notification.html:34
> + window.testRunner.notifyDone();</span >
use finishJSTest() instead of notifyDone
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-multiple-liveregions-notification.html:40
> + window.testRunner.waitUntilDone();</span >
use window.jsTestIsAsync = true instead of this
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-multiple-liveregions-notification.html:42
> + document.getElementById("liveregion1").focus();</span >
i don't like using the method of focus() then getting focusedElement.
instead use "accessibilityController.accessibleElementById('liveRegion1')
<span class="quote">> LayoutTests/platform/mac/accessibility/aria-multiple-liveregions-notification.html:53
> + operation1();</span >
these should have meaningful names instead of operation*</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>