[webkit-reviews] review granted: [Bug 135074] Clicking on links while accessibility is enabled does not render as expected : [Attachment 235255] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 135074: Clicking on links while accessibility is enabled does not render as
expected
https://bugs.webkit.org/show_bug.cgi?id=135074

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=235255&action=review


It’s gratuitous to include all those nullptr changes in this patch. Should just
land them separately so this patch is small and to the point.

>> Source/WebCore/ChangeLog:26
>> +	    (WebCore::AccessibilityObject::updateBackingStore): Problem 3
above.
> 
> There is no problem 3 above.

Oh, I see, it’s problem 2 above.

>> Source/WebCore/accessibility/AccessibilityObject.cpp:1440
>> +	    updateChildrenIfNecessary();
> 
> Why this hasOneRef() optimization? Is it for correctness or efficiency? Maybe
it’s explained in the mysterious “problem 3” discussion?

Now that I understand “problem 2”, I believe firmly that the hasOneRef
optimization is not helpful. I suggest you omit it for simplicity, even though
it’s kind of neat to save the work. I don’t think we need to optimize this kind
of unusual edge case, just prevent it from crashing.