Page MenuHomePhabricator

evas_textblock : fix text insertion & selection with ps in single line
ClosedPublic

Authored by AbdullehGhujeh on Mar 30 2020, 5:15 AM.

Details

Summary

when we have text that contains <ps> (example "p1<ps>p2") in a single line mode
and the cursor position is after the ps tag
then we try to insert any character using the keyboard it will show segmentation fault.
also with the same text if we try to select the text we will notice that it is corrupted.

this should resolve https://phab.enlightenment.org/T8594

Test Plan
#define EFL_EO_API_SUPPORT 1
#define EFL_BETA_API_SUPPORT 1

#include <Eina.h>
#include <Elementary.h>
#include <Efl_Ui.h>

static void
_gui_quit_cb(void *data EINA_UNUSED, const Efl_Event *event EINA_UNUSED)
{
   efl_exit(0);
}

static void
_gui_setup()
{
   Eo *win, *box;

   win = efl_add(EFL_UI_WIN_CLASS, efl_main_loop_get(),
                 efl_ui_win_type_set(efl_added, EFL_UI_WIN_TYPE_BASIC),
                 efl_text_set(efl_added, "Hello World"),
                 efl_ui_win_autodel_set(efl_added, EINA_TRUE));

   // when the user clicks "close" on a window there is a request to delete
   efl_event_callback_add(win, EFL_UI_WIN_EVENT_DELETE_REQUEST, _gui_quit_cb, NULL);

   box = efl_add(EFL_UI_BOX_CLASS, win,
                efl_content_set(win, efl_added),
                efl_gfx_hint_size_min_set(efl_added, EINA_SIZE2D(360, 240)));

   Eo *text = efl_add(EFL_UI_TEXTBOX_CLASS, box,
           efl_gfx_hint_weight_set(efl_added, 1.0, 1.0),
           efl_gfx_hint_align_set(efl_added, 1.0, 1.0),
           efl_pack(box, efl_added));

           efl_text_interactive_selection_allowed_set(text, EINA_TRUE);
           efl_text_multiline_set(text,EINA_FALSE);

           efl_text_markup_set(text, "p1<ps>p2");  
}

EAPI_MAIN void
efl_main(void *data EINA_UNUSED, const Efl_Event *ev EINA_UNUSED)
{
   _gui_setup();
}
EFL_MAIN()

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
AbdullehGhujeh created this revision.Mar 30 2020, 5:15 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

AbdullehGhujeh requested review of this revision.Mar 30 2020, 5:15 AM
AbdullehGhujeh edited the summary of this revision. (Show Details)Mar 30 2020, 5:28 AM
AbdullehGhujeh edited the test plan for this revision. (Show Details)
AbdullehGhujeh added a reviewer: ali.alzyod.
AbdullehGhujeh edited the test plan for this revision. (Show Details)
ali.alzyod updated this revision to Diff 29772.Mar 31 2020, 3:34 AM
  • add test case
ali.alzyod requested changes to this revision.Mar 31 2020, 3:34 AM
This revision now requires changes to proceed.Mar 31 2020, 3:34 AM

Testing is failing because text nodes don't recreated on efl_multiline_set.
in simple words

Fails

efl_text_markup_set(txt, "p1<ps/>p2<ps/>p3");
efl_text_multiline_set(txt, EINA_FALSE);

Works fine

efl_text_multiline_set(txt, EINA_FALSE);
efl_text_markup_set(txt, "p1<ps/>p2<ps/>p3")
  • fix to work when changing mode & add more tests

Can you rebase please, it is important to fix issues related to clipboard

rebase + remove empty line

ali.alzyod requested changes to this revision.Apr 12 2020, 1:23 AM

ninja test fails

90%: Checks: 11, Failures: 0, Errors: 1
../src/tests/elementary/efl_ui_test_text.c:420:E:efl_ui_text:text_multiline_singleline_cursor_pos:0: (after this point) Received signal 11 (Segmentation fault)
This revision now requires changes to proceed.Apr 12 2020, 1:23 AM

fix an issue with tests

ali.alzyod added inline comments.Apr 12 2020, 8:06 AM
src/lib/evas/canvas/evas_object_textblock.c
17032

Is this line needed ?

17100

How about check format nodes for paragraph separator first?
If there is no PS nodes then skip this process.

17106

I think, for first paragraph there are unneeded process

remove uneeded line + check for ps on start + skip uneeded process for first paragraph

AbdullehGhujeh marked 3 inline comments as done.Apr 13 2020, 2:11 AM
AbdullehGhujeh added inline comments.
src/lib/evas/canvas/evas_object_textblock.c
17032

removed

17100

I have updated the code to check.

17106

I stopped looking for cursors to update if we are in the first paragraph

ali.alzyod added inline comments.Apr 14 2020, 12:39 PM
src/lib/evas/canvas/evas_object_textblock.c
17113

here still, most of the first node is useless, we keep assign same format nodes

17115

you can process first node before the loop (you only need len)
Also you can get ride of this check too,

This comment was removed by ali.alzyod.
ali.alzyod added inline comments.Apr 14 2020, 10:25 PM
src/lib/evas/canvas/evas_object_textblock.c
17062

can you use cursor instead of data

17180

can you use cursor instead of data

ali.alzyod added inline comments.Apr 14 2020, 10:29 PM
src/lib/evas/canvas/evas_object_textblock.c
17016

How about change return value to BOOL ?

17086

How about change return value to BOOL ?

AbdullehGhujeh marked 5 inline comments as done.

change return type + change variable name + move code for first par out of loop

AbdullehGhujeh marked 4 inline comments as done.Apr 15 2020, 2:19 AM
AbdullehGhujeh added inline comments.
src/lib/evas/canvas/evas_object_textblock.c
17016

changed

17062

changed

17086

changed

17115

code moved out of the loop

17180

changed

AbdullehGhujeh marked 3 inline comments as done.

move set dirty after if statement

change return type to void

remove comment

  • update code optimization/test cases
ali.alzyod accepted this revision.EditedApr 16 2020, 12:09 AM

This patch seems fine to me, and It fix dealing with PS in single-line mode.

This revision is now accepted and ready to land.Apr 16 2020, 12:09 AM
woohyun accepted this revision.Apr 16 2020, 4:02 AM

This patch seems to fix the issue well :) Good job !

This revision was automatically updated to reflect the committed changes.