[webkit-reviews] review requested: [Bug 100275] Programmatically-inserted children lack accessibility events : [Attachment 218683] Patch proposal

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Dec 8 04:53:52 PST 2013


Mario Sanchez Prada <mario at webkit.org> has asked  for review:
Bug 100275: Programmatically-inserted children lack accessibility events
https://bugs.webkit.org/show_bug.cgi?id=100275

Attachment 218683: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=218683&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
Chris, would you mind partly reviewing the patch again before I landed it?

The reason for that is that I'm now in the WebKitGTK+ Hackfest[1] and both
Joanie and me realized it would be a good idea to add an extra check before
emitting the children-changed::add signal, to avoid getting notifications about
objects that we can already (and quickly) now in advance won't get exposed.

The only difference with the previous patch is that I added this check in
attachWrapper():

+    // Don't emit the signal for objects that we already know won't be exposed

+    // directly for ATK. we don't want to use
accessibilityObjectIsIgnoredByDefault()
+    // or other related methods (e.g. accessibilityIsIgnored()) here because
those might
+    // trigger the execution of code that might be dangerous or just
computationally
+    // excessive at this stage (creating new objects by traversing the
accessibility tree).
+    AccessibilityObject* coreParent = obj->parentObjectUnignored();
+    if (!coreParent || coreParent->accessibilityPlatformIncludesObject() ==
IgnoreObject)
+	 return;
+

This addition alone prevents WebKitGTK+ from emitting a whole lot of signals
about irrelevant objects in the webchat use case, leaving only those ones that
matter, so I think it's a thing we should definitely add.

The other difference with the previous patch is that I removed the
waitUntilDone() bit in the layout test.

Thanks, and sorry for bothering you one more time with this.

[1] https://wiki.gnome.org/Hackfests/WebKitGTK2013


More information about the webkit-reviews mailing list