Page MenuHomePhabricator

edje_entry: fix some preediting bugs
ClosedPublic

Authored by woohyun on Thu, Jan 9, 5:53 AM.

Details

Summary
  1. Attributes can come with random sequence. So, attribute list should be sorted based on start_index.
  2. None tag can be used for some languages' preediting. So, the tag also needs to be handled the same with other tags.

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.
woohyun created this revision.Thu, Jan 9, 5:53 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/

woohyun requested review of this revision.Thu, Jan 9, 5:53 AM
woohyun updated this revision to Diff 28069.Thu, Jan 9, 8:42 PM

eina_list_sort only when attrs is not null

woohyun updated this revision to Diff 28070.Thu, Jan 9, 8:46 PM

just removed a meaningless space

Can you please add some examples or showcase for issues, before and after this patch?

@ali.alzyod

Sorry for giving little information about the patch.
This patch is for the case when preediting string comes with "none" tag and random start index.

For example,

  1. Input "S" (preediting string = "<preedit>S</preedit>")
  2. Input "a" (preediting string = "S<preedit>a</preedit>" and "Sa" should be in the elm_entry)

[With the problematic code]
At the step 2, in the loop of "EINA_LIST_FOREACH(attrs, l, attr)" , let's assume that "a" comes with "preedit tag" and "attr->start_index = 1" first.
Then, "eina_strbuf_append_printf(buf, "<%s>%s</%s>", tagname[attr->preedit_type], markup_txt, tagname[attr->preedit_type]);" will append "<preedit>a</preedit>" to the buf.

In the next loop, let's assume that "S" comes with "none tag" and "attr->start_index=0".
Then, "eina_strbuf_append(buf, preedit_string);" will append whole preediting string (="Sa") to existing buffer (="<preedit>a</preedit>").
So, the result is "<preedit>a</preedit>Sa".

[With the patch]

  1. "attrs" list is sorted based on the "attr->start_index". So, "S" will come first (i.e. earlier than "a").
  2. "none" tag will not append whole preediting string. So, only "S" will be appended to the buffer.

So, the expected result "S<preedit>a</preedit>" can some after the patch.

If my explanation would not be good enough for your understanding, please let me know.

bu5hm4n requested changes to this revision.Mon, Jan 13, 3:16 AM
bu5hm4n added a subscriber: bu5hm4n.
bu5hm4n added inline comments.
src/lib/edje/edje_entry.c
4874

Can that ever really happen ? If not, then we should probebly have a EINA_SAFETY.

4877

The equal case is missing.

This revision now requires changes to proceed.Mon, Jan 13, 3:16 AM
woohyun marked 2 inline comments as done.Tue, Jan 14, 1:21 AM
woohyun updated this revision to Diff 28153.Tue, Jan 14, 1:22 AM

update following @bu5hm4n's comment

woohyun added inline comments.Tue, Jan 14, 1:25 AM
src/lib/edje/edje_entry.c
4874

I did add EINA_SAFETY_ON_NULL_RETURN_VAL. Did I catch your point correctly ?

bu5hm4n accepted this revision.Tue, Jan 14, 1:52 AM

Cool - yeah you catched everything :)

This revision is now accepted and ready to land.Tue, Jan 14, 1:52 AM
Closed by commit rEFL04d4aeb8b78c: edje_entry: fix some preediting bugs (authored by woohyun, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyTue, Jan 14, 8:21 AM
This revision was automatically updated to reflect the committed changes.