[webkit-reviews] review granted: [Bug 132527] AX: [ATK] [PATCH] add text-caret-moved signal to accessibilityController : [Attachment 232251] caret-moved signal and caret offset tests v2.05

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 30 02:13:49 PDT 2014


Mario Sanchez Prada <mario at webkit.org> has granted Jarek Czekalski
<jarekczek at poczta.onet.pl>'s request for review:
Bug 132527: AX: [ATK] [PATCH] add text-caret-moved signal to
accessibilityController
https://bugs.webkit.org/show_bug.cgi?id=132527

Attachment 232251: caret-moved signal and caret offset tests v2.05
https://bugs.webkit.org/attachment.cgi?id=232251&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=232251&action=review


Thanks for the new patch, it's almost there. Only a minor suggestion (in the
ChangeLog, sorry!) and I think we are ready to give this a try.

(In reply to comment #16)
> (From update of attachment 231629 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=231629&action=review
> [...]
> The comments were outdated indeed. Have a look at them once more before
committing, because I rewrote them.

I think the new comments are fine

> LayoutTests/ChangeLog:30
> +	   * platform/gtk/accessibility/caret-offsets.html: Added.
> +	   * platform/gtk/accessibility/caret-offsets-expected.txt: Added.

You forgot to mention the new test
caret-offsets-and-extraneous-white-spaces.html here. But that's the only thing
I see in this patch at the moment so iy fyou could update this changelog and
submit a new patch just with that change, I'll r+ and cq+ rightaway.

By the way, thanks a lot for splitting the original test in two anyway.


More information about the webkit-reviews mailing list