[webkit-reviews] review granted: [Bug 72830] [Gtk] Regression: text-inserted events lack text inserted and current line : [Attachment 121274] Patch proposal for the remaining issues + updated unit tests

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 72830: [Gtk] Regression: text-inserted events lack text inserted and
current line
https://bugs.webkit.org/show_bug.cgi?id=72830

Attachment 121274: Patch proposal for the remaining issues + updated unit tests
https://bugs.webkit.org/attachment.cgi?id=121274&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=121274&action=review


Looks good, but consider the following changes.

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2717
> +AccessibilityObject*
focusedObjectAndCaretOffsetUnignored(AccessibilityObject* objectOfReference,
int& offset)

You can just call objectOfReference referenceObject, if you like.

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2739
> +    AccessibilityObject* realObject = focusedObject;

I think I would prefer just continuing to use focusedObject here. After this
point it seems unused and realObject is a little ambiguous.

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.h:63
> +WebCore::AccessibilityObject*
focusedObjectAndCaretOffsetUnignored(WebCore::AccessibilityObject*
objectOfReference, int& offset);

objectFocusedAndCaretOffsetUnignored might be clearer. You can just omit the
parameter name here instead of calling it objectOfReference.