Bug 241915

<rdar://problem/95776167>

Created attachment 460444 [details] Patch

Comment on attachment 460444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460444&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:367 > + ASSERT(is<AccessibilityObject>(axObject)); I don't know if we need to assert here, because it will crash on the next line anyway

Comment on attachment 460444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460444&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:1913 > + handleTextChanged(getOrCreate(element)); I'm worried about the getOrCreate() here. this might get fired during layout and creating ax objects can be problematic. it looks like the rest of the versions here only use get() or defer changes

(In reply to chris fleizach from comment #4) > Comment on attachment 460444 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=460444&action=review > > > Source/WebCore/accessibility/AXObjectCache.cpp:1913 > > + handleTextChanged(getOrCreate(element)); > > I'm worried about the getOrCreate() here. this might get fired during layout > and creating ax objects can be problematic. it looks like the rest of the > versions here only use get() or defer changes That's what we had before for textChanged: -void AXObjectCache::textChanged(Node* node) { - textChanged(getOrCreate(node)); -} I could change that, but didn't want to change a long standing behavior this late.

Comment on attachment 460444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460444&action=review >>> Source/WebCore/accessibility/AXObjectCache.cpp:1913 >>> + handleTextChanged(getOrCreate(element)); >> >> I'm worried about the getOrCreate() here. this might get fired during layout and creating ax objects can be problematic. it looks like the rest of the versions here only use get() or defer changes > > That's what we had before for textChanged: > > -void AXObjectCache::textChanged(Node* node) > { > - textChanged(getOrCreate(node)); > -} > > I could change that, but didn't want to change a long standing behavior this late. if its the same behavior we can probably keep as is but could be problematic in the future

Created attachment 460452 [details] Patch

(In reply to chris fleizach from comment #3) > Comment on attachment 460444 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=460444&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:367 > > + ASSERT(is<AccessibilityObject>(axObject)); > > I don't know if we need to assert here, because it will crash on the next > line anyway Removed assert.

(In reply to chris fleizach from comment #6) > Comment on attachment 460444 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=460444&action=review > > >>> Source/WebCore/accessibility/AXObjectCache.cpp:1913 > >>> + handleTextChanged(getOrCreate(element)); > >> > >> I'm worried about the getOrCreate() here. this might get fired during layout and creating ax objects can be problematic. it looks like the rest of the versions here only use get() or defer changes > > > > That's what we had before for textChanged: > > > > -void AXObjectCache::textChanged(Node* node) > > { > > - textChanged(getOrCreate(node)); > > -} > > > > I could change that, but didn't want to change a long standing behavior this late. > > if its the same behavior we can probably keep as is but could be problematic > in the future Agree.

Created attachment 460471 [details] Patch

Committed 251817@main (6dfa4f8dfed6): <https://commits.webkit.org/251817@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 460471 [details].