RESOLVED WONTFIX 44334
[EFL] Text selection advanced API
https://bugs.webkit.org/show_bug.cgi?id=44334
Summary [EFL] Text selection advanced API
Mirek Szymanski
Reported 2010-08-20 06:49:16 PDT
This patch contains new methods in EFL port to control text selection: - select a word under given position - return positions of upper left/bottom right corner of the selection - modify left/right position of the selection - check if there is selection at given position Regards, Miroslaw
Attachments
Implement API to manage text selection (9.39 KB, patch)
2010-08-20 07:06 PDT, Mirek Szymanski
no flags
Patch (9.40 KB, patch)
2011-03-03 21:50 PST, Ryuan Choi
no flags
Patch (9.29 KB, patch)
2011-06-01 07:32 PDT, Ryuan Choi
no flags
Mirek Szymanski
Comment 1 2010-08-20 07:06:29 PDT
Created attachment 64954 [details] Implement API to manage text selection
Rafael Antognolli
Comment 2 2010-08-26 09:52:15 PDT
Hi Mirek, The patch is nice, but could you please consider adding some doxygen comments on each of these functions, describing the parameters and return values? I also couldn't get the reason why you need top_h and bottom_h parameters, but I think the documentation will be enough for me to understand it. Another change if you want to make the API more versatile is here: +Eina_Bool ewk_frame_selection_handlers_get( + Evas_Object* o, + int* top_x, int* top_y, int* top_h, + int* bottom_x, int* bottom_y, int* bottom_h) +{ + EWK_FRAME_SD_GET_OR_RETURN(o, sd, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(sd->frame, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(top_x, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(top_y, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(top_h, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(bottom_x, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(bottom_y, EINA_FALSE); + EINA_SAFETY_ON_NULL_RETURN_VAL(bottom_h, EINA_FALSE); Instead of failing if any of the parameters is NULL, you can check each of them before assigning its value, like the function ewk_view_viewport_get(). This would allow the user to fetch just some of the values, without the need to create dummy variables to receive the other parameters. This comment is valid for all these new functions you've created. Regards, Rafael
Mirek Szymanski
Comment 3 2010-08-27 00:59:34 PDT
Thanks Rafael for the comments. I'll get back to this patch later, when I'm back from vacations. -Mirek
Adam Barth
Comment 4 2010-08-30 16:31:16 PDT
Comment on attachment 64954 [details] Implement API to manage text selection Clearing the review flag since miroslaw.s will get back to this patch later.
Lucas De Marchi
Comment 5 2011-01-06 09:23:03 PST
Hi, Mirek (In reply to comment #3) > Thanks Rafael for the comments. I'll get back to this patch later, when I'm back from vacations. > Are you planning to re-submit this?
Ryuan Choi
Comment 6 2011-03-03 21:50:24 PST
Ryuan Choi
Comment 7 2011-03-03 21:58:46 PST
(In reply to comment #6) > Created an attachment (id=84688) [details] > Patch Instead of mirek, I upload patch including rafael's comment. Especially, I removed top_h and bottom_h parameters because it's special case we are using.
Ryosuke Niwa
Comment 8 2011-04-11 15:59:39 PDT
Comment on attachment 84688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84688&action=review > Source/WebKit/efl/ewk/ewk_frame.cpp:761 > + selectedRange->textRects(rects, true); selectedRange can be null > Source/WebKit/efl/ewk/ewk_frame.cpp:785 > +Eina_Bool ewk_frame_select_closest_word(Evas_Object* o, int x, int y, Eina_Rectangle* left_handle, Eina_Rectangle* right_handle) This function seems to duplicate what EventHandler::selectClosestWordFromMouseEvent implements. Can we share code with that function somehow?
Ryosuke Niwa
Comment 9 2011-04-11 16:00:14 PDT
Comment on attachment 84688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84688&action=review >> Source/WebKit/efl/ewk/ewk_frame.cpp:761 >> + selectedRange->textRects(rects, true); > > selectedRange can be null r- because of this.
Ryuan Choi
Comment 10 2011-04-11 17:42:21 PDT
(In reply to comment #9) > (From update of attachment 84688 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84688&action=review > > >> Source/WebKit/efl/ewk/ewk_frame.cpp:761 > >> + selectedRange->textRects(rects, true); > > > > selectedRange can be null > > r- because of this. Thank you for your review. I'll fix and check EventHandler::selectClosestWordFromMouseEvent.
Ryuan Choi
Comment 11 2011-06-01 07:32:33 PDT
Ryuan Choi
Comment 12 2011-06-01 07:46:40 PDT
(In reply to comment #9) > (From update of attachment 84688 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84688&action=review > > >> Source/WebKit/efl/ewk/ewk_frame.cpp:761 > >> + selectedRange->textRects(rects, true); > > > > selectedRange can be null Done. > > r- because of this. Despite of your kindness, I can't find easy way to use EventHandler::selectClosestWordFromMouseEvent on WebKit/EFL without events.
Rafael Antognolli
Comment 13 2011-06-01 08:59:56 PDT
(In reply to comment #12) > (In reply to comment #9) > > (From update of attachment 84688 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=84688&action=review > > > > >> Source/WebKit/efl/ewk/ewk_frame.cpp:761 > > >> + selectedRange->textRects(rects, true); > > > > > > selectedRange can be null > Done. > > > > > r- because of this. > > Despite of your kindness, > I can't find easy way to use EventHandler::selectClosestWordFromMouseEvent on WebKit/EFL without events. Hello Ryuan, I think Ryosuke isn't talking about using EventHandler::selectClosestWordFromMouseEvent directly, but in someway refactor that function so it would call another function. This other function would have the common code available to be called by ewk_frame_select_closest_word.
Gyuyoung Kim
Comment 14 2011-06-19 00:23:38 PDT
Ryuan, This bug is too old. Any news on this bug ?
Kenneth Rohde Christiansen
Comment 15 2011-09-12 15:32:40 PDT
Comment on attachment 95599 [details] Patch I don't see why you ned to export such an API. In Qt (not all upstreamed) we don't do that and we have quite good selection/composition support for our virtual keyboard.
Ryuan Choi
Comment 16 2011-09-13 16:38:46 PDT
(In reply to comment #15) > (From update of attachment 95599 [details]) > I don't see why you ned to export such an API. In Qt (not all upstreamed) we don't do that and we have quite good selection/composition support for our virtual keyboard. OK, I'll check and try to find proper way. I'll close this until I find better way.
Note You need to log in before you can comment on or make changes to this bug.