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
Created attachment 64954 [details] Implement API to manage text selection
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
Thanks Rafael for the comments. I'll get back to this patch later, when I'm back from vacations. -Mirek
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.
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?
Created attachment 84688 [details] Patch
(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.
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?
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.
(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.
Created attachment 95599 [details] Patch
(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.
(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.
Ryuan, This bug is too old. Any news on this bug ?
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.
(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.