Bug 114870 – [GTK] Reimplement atk_text_get_text_*_offset for CHAR boundary

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Mario Sanchez Prada]

We need to get rid of Pango/Gail dependency, and this is one of the functionalities we would need to fix.

[Mario Sanchez Prada]

Created attachment 201174 [details]
Patch proposal

Patch proposal for this use case. Additional tests are not needed since we are replacing and old implementation, and so this functionality is tested by the testWebkitAtkGetTextAtOffset*() family of functions inside testatk.c

[Mario Sanchez Prada]

Created attachment 201175 [details]
Patch proposal

This is the right one. Seriously.

[EFL EWS Bot]

Comment on attachment 201175 [details]
Patch proposal

Attachment 201175 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/302114

[EFL EWS Bot]

Comment on attachment 201175 [details]
Patch proposal

Attachment 201175 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/416575

[Mario Sanchez Prada]

Created attachment 202422 [details]
Patch proposal

This new patch avoids defining that function for the EFL platform

[Martin Robinson]

Not sure I understand why this is still behind an #ifdef is the code can be shared now. Here's the compilation problem on EFL, I think:

[ 13%] Building CXX object Source/WebCore/CMakeFiles/webcore_efl.dir/accessibility/atk/WebKitAccessibleInterfaceText.cpp.o
/mnt/eflews/git/webkit/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:609:15: warning: unused parameter 'textPosition' [-Wunused-parameter]
/mnt/eflews/git/webkit/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:578:15: error: 'gchar* webkitAccessibleTextGetChar(AtkText*, gint, GetTextRelativePosition, gint*, gint*)' defined but not used [-Werror=unused-function]
cc1plus: all warnings being treated as errors
make[2]: *** [Source/WebCore/CMakeFiles/webcore_efl.dir/accessibility/atk/WebKitAccessibleInterfaceText.cpp.o] Error 1
make[1]: *** [Source/WebCore/CMakeFiles/webcore_efl.dir/all] Error 2

[Mario Sanchez Prada]

(In reply to comment #6)
> Not sure I understand why this is still behind an #ifdef is the code can be shared now.

I just kept them there not to break EFL behaviour (even if so far it's just "being not implemented"), but I'm currently talking to EFL guys to see what it's best. Probably sharing it, as you mentioned.

In any case, another open question is whether we should move WebCore/accessibility to WebCore/platform/accessibility since, after all, a11y layer is pretty much a platform dependent thing.

Here's the compilation problem on EFL, I think:
> 
> [ 13%] Building CXX object Source/WebCore/CMakeFiles/webcore_efl.dir/accessibility/atk/WebKitAccessibleInterfaceText.cpp.o
> /mnt/eflews/git/webkit/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:609:15: warning: unused parameter 'textPosition' [-Wunused-parameter]
> 

Argh! I should rebase my local branch it seems, to pick Edu's patch.

/mnt/eflews/git/webkit/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:578:15: error: 'gchar* webkitAccessibleTextGetChar(AtkText*, gint, GetTextRelativePosition, gint*, gint*)' defined but not used [-Werror=unused-function]
> cc1plus: all warnings being treated as errors
> make[2]: *** [Source/WebCore/CMakeFiles/webcore_efl.dir/accessibility/atk/WebKitAccessibleInterfaceText.cpp.o] Error 1
> make[1]: *** [Source/WebCore/CMakeFiles/webcore_efl.dir/all] Error 2

That's what this last patch should have fixed.

[Mario Sanchez Prada]

Created attachment 202426 [details]
Patch proposal

I talked to EFL guys and they do not see any problem in sharing this implementation, now it's free from Pango/Gail stuff, so here it's the patch.

About the other bits (moving stuff into WebCore/platform), I'll send an email about that to discuss it properly.

[Martin Robinson]

Comment on attachment 202426 [details]
Patch proposal

View in context: https://bugs.webkit.org/attachment.cgi?id=202426&action=review

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:604
> +    if (isEndOfLine(visPos))

Please use visiblePosition here for the variable name.

[Mario Sanchez Prada]

Committed r150518: <http://trac.webkit.org/changeset/150518>