With RTL (Arabic) format, if a contact does have a too long e-mail address, it will truncate the beginning of the address instead of the end. Flame 3.0: Build ID 20150126010231 Build Type user Gaia Revision 0f662dffef27599443cfcd790c2b39190a2b35c8 Gaia Date 2015-01-25 00:48:56 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/fa91879c8428 Gecko Version 38.0a1 Device ID flame Firmware(Release) 4.4.2 Firmware(Incremental) 39 Firmware Date Thu Oct 16 18:19:14 CST 2014 Bootloader L1TC00011880

Seems to exist at least for Settings too, so probably to other places into the OS. Depends on bug 1126606.

I see the same behavior in English. Would you mind checking it too?

in RTL language, truncation is expected to be at the "beginning" of a word since this that's where the word ends. Not sure this can be changed for when using non-RTL characters within the RTL behaviour. Will close this one as INVALID but keep an eye out on Bugs such as Bug 1126883,since that addresses the LTR case

(In reply to Delphine Lebédel [:delphine - use need info] from comment #4) > in RTL language, truncation is expected to be at the "beginning" of a word > since this that's where the word ends. > Not sure this can be changed for when using non-RTL characters within the > RTL behaviour. > Will close this one as INVALID but keep an eye out on Bugs such as Bug > 1126883,since that addresses the LTR case No, it seems this can be determined. I discussed the problem with julienw two days ago. It seems they solved this problem in the Messages app by using an automatic property for these elements instead of manually setting it. That way, the behavior (truncated side) will change depending on the nature of the string. Anyway, I NI him in case I would say a mistake.(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #3) > I see the same behavior in English. Would you mind checking it too? You mean, RTL english? (If yes, I don't have it on my build), because currently: - English have right side truncated, which is the end for occidental characters strings; - Arabic have left side truncated, but considering it's here occidental characters strings (e-mails for example), it truncated the beginning of the string.

we used "dir=auto" in the SMS app; you can try inputting an email as recipient in the "new message" panel you'll see it's properly truncated.

(In reply to Clément Lefèvre from comment #5) > [:jlorenzo] (QA) from comment #3) > > I see the same behavior in English. Would you mind checking it too? > > You mean, RTL english? I should have taken a screenshot. You're right I don't see anything suspicious anymore. I might have hit another bug while switching LTR/RTL languages on the fly.

I meant regular English.

(In reply to Clément Lefèvre from comment #0) > With RTL (Arabic) format, if a contact does have a too long e-mail address, > it will truncate the beginning of the address instead of the end. This is why I closed this bug as resolved invalid. In Arabic language, what you describe is what is expected. Truncation should happen on the left of the word instead of the right. If this bug is not invalid and you meant in RTL BUT using non-RTL language, then let's reopen this. Leaving as is at the time, feel free to take action on it.

(In reply to Delphine Lebédel [:delphine - use need info] from comment #9) > (In reply to Clément Lefèvre from comment #0) > > With RTL (Arabic) format, if a contact does have a too long e-mail address, > > it will truncate the beginning of the address instead of the end. > > This is why I closed this bug as resolved invalid. In Arabic language, what > you describe is what is expected. Truncation should happen on the left of > the word instead of the right. > If this bug is not invalid and you meant in RTL BUT using non-RTL language, > then let's reopen this. > Leaving as is at the time, feel free to take action on it. Yes I meant in RTL mode (like Arabic) but for LTL strings, as I was talking about e-mail addresses for example. This result on the beginning of the string being truncated instead of the end. I'll join two screenshots, for edit mode and for the other mode and for contact list. About reopening the bug, I NI you Delphine, as I does not have the rights on BMO to do so.

Ok reopening, thanks for clarifying Clément! I would assume this would probably have to follow the same rules and that Ahmed shows here: https://bug1126900.bugzilla.mozilla.org/attachment.cgi?id=8560153

If someone could check in the phone call history too ; I do not have a SIM card right now in the phone to check this and maybe it does have the same effet on long name or long numbers. In the opposite case, NI me and I'll test it with a SIM card later.

FWIW here is how it works in the SMS app in RTL mode (but it should be the same in LTR mode actually -- basically this should only depend on the content, not on the language). I think it's how it should work everywhere as well.

triage: agree with comment #15 that we should follow this pattern in the other apps.

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master Hi Jose, Could you review this please? Clément, FYI this also affect other apps as well. So you might want to check new behavior when this patch lands.

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master Hi Augustin, I would prefer this kind of changes to be reviewed by a BB owner. thanks!

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master This is more for Pavel :)

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master I agree with Julien that Pavel might be a better reviewer. But since I have already started my review, I'm going to complete it. First, I am the module owner for the shared/ module, but in reality only for shared/js. We apparently don't have a module owner for shared/style, and I do not know who any of the people are who review patches to these files. Pavel seems like a good choice. Having said that, however, I'd give this patch a r- for these reasons: 1) a generic change like this to shared code this late in the 2.2 cycle seems risky. I'd prefer to patch the individual apps that we know are affected and not risk causing regressions to other apps (There might be apps that use a bdi element in a list but do not want display:inline-block, for example.) And since we are moving toward using web components on master, there isn't much long term value in making improvements to the share stylesheets. I'd say that the risk of this shared change outweighs the benefit. 2) The contacts app seems to use "li > p > bdi" for these contact names and email addresses. The list.css file already has styles for "li p". And you want to add "li bdi" that duplicates the overflow properties. This seems too general. Can you use "li > p > bdi:only-child"? If that works, it seems specific enough that it would be much safer. It would still be easier to get reviews done if you made this change in app-specific stylesheets instead of shared stylesheets. 3) You could also try the approach Julien suggested and just set .dir="auto" on the relevant P elements in the contacts app. 4) For the display of contact names note that the bdi element contains indivdiually styled given name and family name. Will your fix work correctly in that case? The third screenshot you attached appears to show a single long string rather than a two part name, so be sure to test with actualy two-part names. 5) In comment 18, you mention that this patch affects other apps. If you really want to land a change to the shared file, please determine which other apps are affected by it and list them here. Don't just leave that up to QA.

+1 for David comment

Thanks for this very complete review! I will update my patch accordingly (not sure it will be in time for 2.2 but I'll try). Sorry for the mistake in reviewers, I didn't know you were only module owner for shared/js. Will try to remember that! Note that for 3), dir="auto" does not work here because we still want the text to be right-aligned (in RTL languages) regardless of the actual content (RTL or LTR string). That's why we can't use dir="auto". We need to inherit dir rtl or ltr. Otherwise, what you told me makes really sense to me, thanks!

Augustin did you provide a new patch? In that case you should flag again for review.

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master Oh thanks, I forgot to flip the r flag again!

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master I think David is more familiar with this code :)

Actually no, it is not ready yet because there is still the title of contact detail that is not right.

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master So this patch correct the ellipsis in the list and in the contact detail page (the email field). However it does not correct the ellipsis in the title of the contact detail page. This is done by the gaia-header component. To fix this, my only guess right now would be to add a dir="auto" to the h1 and remove the direction="rtl" or "ltr" in the css, but that is too much for me to try without some advice :-)

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master This is a contacts app fix, and looking at the module owners page: https://wiki.mozilla.org/Modules/FirefoxOS It looks like Francisco might be module owner there, so I will route the review to him, since I do not have owner/peer status for that module.

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master Tried and working perfectly, thanks!

Please ask for approval 2.2 once this lands on master

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/78c56628848a3bc71886328035da1aa3557df24e

Comment on attachment 8566496 [details] [gaia] autra:bug-1126587 > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): rtl support [User impact] if declined: user won't see the correct side of email adress, which can hinder readability of contact list [Testing completed]: on flame v2.2 [Risk to taking this patch] (and alternatives if risky): low [String changes made]: none

The landing on master caused bug 1139185 so I don't know if you want to uplift this to 2.2.

(In reply to KTucker [:KTucker] from comment #35) > The landing on master caused bug 1139185 so I don't know if you want to > uplift this to 2.2. Looks like the follow-up is up for review. I'll wait till that's fixed on 3.0 as well so both these can be uplifted at once..

v2.2: https://github.com/mozilla-b2g/gaia/commit/ab4a3ac1407bbf66cf552fbc06a200b562d0f634

Hi clement, During verifying this bug, these cases mentioned on attachments 8560179 8560180 8560182, are fixed now, but I find another case, please see attachment, when you are watching one contact which is with long LTR string name, the name shown on header bar will be truncated at the beginning. And I am also doubt another case: whether long number should be truncated at begining or ending, please also see attachment. Could you confirm this? and may I reopen this bug until these two cases are fixed? Thanks.

This issue is verified fixed on the latest Nightly Flame 3.0 and 2.2 builds. Actual Results: Long names and emails written in LTR languages are correctly truncated when the device is set to RTL. Environmental Variables: Device: Flame 3.0 KK (319MB) (Full Flash) BuildID: 20150313010238 Gaia: eabe35cf054d47087b37c1ca7db8143717fbd7f3 Gecko: 42afc7ef5ccb Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429 Version: 39.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0 Environmental Variables: Device: Flame 2.2 KK (319MB) (Full Flash) BuildID: 20150313002507 Gaia: 4aefc3f6f30a40ac67fdf841b7c90cd648b85369 Gecko: 049713f3b0ed Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429 Version: 37.0 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

@Jayme Mercado: Is it possible that there is any difference between OTA updates and the one you tested? Because on latest nightly by OTA, I'm able to reproduce what Norry.L.F is talking about and so to say that the bug isn't 100% fixed right now. @Norry.L.F: As mentioned above, I can reproduce what you're pointing out. Maybe should we create contact with way more informations filled to check every possible information? By the way, if someone could create a vCard with arabic fields inside to test, it could be fine too (no arabic keyboard to type some random things…) NI on both of you as I thing the bug should be reopened.

Please file a separate bug for the header issue as this is clearly different issues. See also bug 1138340 for a similar issue in the SMS app. For the Phone number issue, I think it should have been fixed here as well, but let's file a separate bug as this will make things easier.

With Julien's suggestions, we submit two bugs. For the header issue, link to this new issue: bug 1143594 For the number issue, link to this new issue: bug 1143599, and it also descrips another two colums in edit screen: company & street. And we have tried to create a contact with arabic fields inside, and it looks fine except for number filed, please see attachment. Build ID 20150315162500 Gaia Revision a6b2d3f8478ec250beb49950fecbb8a16465ff6f Gaia Date 2015-03-15 14:33:22 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/18619f8f6c5c Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150315.195030 Firmware Date Sun Mar 15 19:50:42 EDT 2015 Bootloader L1TC000118D0

Cleaning needinfo, I CC'd me on the two other bugs, I guess that there are nothing to see here anymore as there are bugs opened for them.

Comment 41 and 42 make the ni? on me irrelevant.