When the accessibility tree is turned on, we trigger a synchronous layout in HTMLTextFormControlElement::setInnerTextValue. We need to avoid that. <rdar://problem/32686777>
Created attachment 314001 [details] WIP
Created attachment 314002 [details] WIP
Created attachment 314067 [details] Fixes the bug
Comment on attachment 314067 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=314067&action=review > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:699 > + auto *cache = m_object->axObjectCache(); we probably shouldn't assert cache here, it is possible to have a null cache (happens when our wrapper outlives the backing object), in which can we should return a nil value
Comment on attachment 314067 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=314067&action=review >> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:699 >> + auto *cache = m_object->axObjectCache(); > > we probably shouldn't assert cache here, it is possible to have a null cache (happens when our wrapper outlives the backing object), in which can we should return a nil value Okay. I can fix that.
Created attachment 314070 [details] Addressed Chris's comment
Comment on attachment 314070 [details] Addressed Chris's comment View in context: https://bugs.webkit.org/attachment.cgi?id=314070&action=review small suggestions. otherwise change looks ok to me > Source/WebCore/accessibility/AXObjectCache.cpp:2170 > + textMarkerData.offset = 0; these few lines should be unnecessary now that you initialize them > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:412 > + if (position.isNotNull()) { should use early return here
Created attachment 314074 [details] Patch for landing
Thanks for the review! (In reply to chris fleizach from comment #7) > Comment on attachment 314070 [details] > Addressed Chris's comment > > View in context: > https://bugs.webkit.org/attachment.cgi?id=314070&action=review > > small suggestions. otherwise change looks ok to me > > > Source/WebCore/accessibility/AXObjectCache.cpp:2170 > > + textMarkerData.offset = 0; > > these few lines should be unnecessary now that you initialize them Removed. > > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:412 > > + if (position.isNotNull()) { > > should use early return here Fixed.
Comment on attachment 314074 [details] Patch for landing Wait for EWS.
Created attachment 314075 [details] Patch for landing
Comment on attachment 314075 [details] Patch for landing Wait for EWS again.
Created attachment 314085 [details] Another GTK+/WPE build fix
Created attachment 314087 [details] Patch for landing
Committed r218910: <http://trac.webkit.org/changeset/218910>
(In reply to Ryosuke Niwa from comment #15) > Committed r218910: <http://trac.webkit.org/changeset/218910> I am able to hit ASSERT(LayoutDisallowedScope::isLayoutAllowed()), added in this commit, whenever I have VoiceOver enabled and update the value of text field. See bug #174559.
(In reply to Daniel Bates from comment #16) > (In reply to Ryosuke Niwa from comment #15) > > Committed r218910: <http://trac.webkit.org/changeset/218910> > > I am able to hit ASSERT(LayoutDisallowedScope::isLayoutAllowed()), added in > this commit, whenever I have VoiceOver enabled and update the value of text > field. See bug #174559. *whenever I have VoiceOver enabled and update the value of a text field from a mouseup event listener.