Page MenuHomePhabricator

efl_ui_text: make password mode work properly
Needs RevisionPublic

Authored by woohyun on May 8 2019, 12:00 AM.

Details

Summary

Previously, efl_ui_text did do nothing when password
mode is set. Now, replacement character "*" is shown when
the mode is set

Test Plan
  1. elementary_test
  2. Efl.Ui.Text Password

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D8856
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11318
Build 8611: arc lint + arc unit
woohyun created this revision.May 8 2019, 12:00 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.May 8 2019, 12:00 AM
woohyun edited the test plan for this revision. (Show Details)May 8 2019, 12:01 AM
ali.alzyod added inline comments.May 8 2019, 12:21 AM
src/lib/elementary/efl_ui_text.c
2329

I think we should check (efl_text_replacement_char_get()) if it is null then use "*"

woohyun added inline comments.May 8 2019, 12:52 AM
src/lib/elementary/efl_ui_text.c
2329

I also considered the case that user set their own replacement characters.
But, user's replacement characters would be lost when password_mode is unset.
(by the below code (=efl_text_replacement_char_set(obj, "");)

I think we can make a new interface such as efl_ui_text_password_char_set();
(because we cannot override efl_text_replacement_char_set (=composition API))
But, here, I only purposed to make password mode is working fine.

ali.alzyod added inline comments.May 8 2019, 1:04 AM
src/lib/elementary/efl_ui_text.c
2329

A quick question why do we we need to clear replacement character efl_text_replacement_char_set(obj, "")

woohyun added inline comments.May 8 2019, 1:21 AM
src/lib/elementary/efl_ui_text.c
2329

Your understanding is right !
I did miss the codes below in evas_object_textblock.

Previously, I thought "password" and "replacement character" are working independently.
But, they are something working together.
I think your indication is right. I'll update the code soon.

4602         if ((queue->format->password) && (repch) && (eina_ustrbuf_length_get(n->unicode)))
4603           {                                                                     
4604              int i, ind;                                                        
4605              Eina_Unicode *ptr;                                                 
4606                                                                                 
4607              tbase = str = ptr = alloca((off + 1) * sizeof(Eina_Unicode));      
4608              ind = 0;                                                           
4609              urepch = eina_unicode_utf8_next_get(repch, &ind);                  
4610              for (i = 0 ; i < off; ptr++, i++)                                  
4611                *ptr = urepch;                                                   
4612              *ptr = 0;                                                          
4613           }
woohyun updated this revision to Diff 22029.May 8 2019, 1:30 AM

remove meaningless code and add more test case

This comment was removed by ali.alzyod.

patch seems fine to me

@ali.alzyod Then accept the revision (in the Add Action... drop down). Welcome as a reviewer!

ali.alzyod accepted this revision.May 8 2019, 2:50 AM
This revision is now accepted and ready to land.May 8 2019, 2:50 AM

@woohyun @segfaultxavi
This is not related directly to this patch, but you can detect the issue here.

efl_ui_text_password_mode_set(T/F), efl_text_password_set(T/F)

I think two interfaces with similar names, it not easy for EFL users, what do you think ?

Can you add this comment to T7849?

@bu5hm4n sure, should I add it directly there or inside sub task

As you wish

@bu5hm4n sure, should I add it directly there or inside sub task

Just add this comment on the task, please. We are now in the process of discussing the API before stabilizing it, and this is exactly the kind of things we need to discuss!

@segfaultxavi sorry I was away from computer, I think you already added the comment.
Thank you

zmike requested changes to this revision.Thu, May 30, 5:02 AM

This needs rebasing.

This revision now requires changes to proceed.Thu, May 30, 5:02 AM