Summary: | [GTK] Reimplement atk_text_get_text_*_offset for CHAR boundary | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mario Sanchez Prada <mario> | ||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, eflews.bot, gyuyoung.kim, jdiggs, mrobinson | ||||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 114867 | ||||||||||||
Attachments: |
|
Description
Mario Sanchez Prada
2013-04-19 06:48:18 PDT
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
Created attachment 201175 [details]
Patch proposal
This is the right one. Seriously.
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 Comment on attachment 201175 [details] Patch proposal Attachment 201175 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/416575 Created attachment 202422 [details]
Patch proposal
This new patch avoids defining that function for the EFL platform
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 (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. 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.
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. Committed r150518: <http://trac.webkit.org/changeset/150518> |