1159982 - [RTL][Email] Long email address is not correctly displayed on email address dialog

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P2)

Description

[Yeojin Chung [:YeojinC]]

Attachment: logcat_20150429_1736.txt

Description:
When a long email address on the recipient field is selected, the animation shows the full email address on the edit dialog. However, in RTL, the animation flows in the wrong direction, and does not show the full email address.

Pre-requisite: Have an email account set up in Email app. Device is set in Arabic.

Repro Steps:
1) Update a Flame to 20150429010205.
2) Open Email app.
3) Select the Compose icon.
4) On "To" field, type any long email address ([e-mail] format), and type in a comma(,).
5) When the truncated email address bubble is created, tap the bubble.
6) Observe the email address at the top of the dialog.


Actual:
The animation flows in a wrong direction, and does not show the full email address.

Expected:
The full email address is shown correctly by the animation.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150429010205
Gaia: 6e35b0948c42a4398b8a5916015de167121683a1
Gecko: 1ad65cbeb2f4
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Repro frequency: 5/5
Link to failed test case:
See attached: video clip, logcathttps://youtu.be/nDnltOqxUgc

Comment 1

[Yeojin Chung [:YeojinC]]

This issue also reproduces on Flame 2.2.

Result: The animation flows in a wrong direction, and does not show the full email address.

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150429002501
Gaia: 1b7aa7e60788668ed09abf76022dfa231dbe88d4
Gecko: d38ff4717f39
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

Comment 2

[Delphine Lebédel [:delphine - Use needinfo]]

[Blocking Requested - why for this release]:

This situation seems like a edge case to me, but on the other hand it's still bad as the email address gets completely reversed, thus losing sense and creating confusion. 
Triage: P2, nominating for 3.0

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] jrburke:bug1159982-email-addr-scroll-dir > mozilla-b2g:master

Comment 4

[James Burke [:jrburke]]

Comment on attachment 8600071 [details] [review]
[gaia] jrburke:bug1159982-email-addr-scroll-dir > mozilla-b2g:master

A couple of factors contributed to this bug:

* The HTML element holding the text did not have dir="auto" set on it, so the layout of the text would be incorrect if, as in the video, an ltr email address is views in an rtl UI.
* The direction of the animation needs to change if the text is actually determined to be rtl.

This pull request fixes both issues: the node holding the text now has dir="auto" on it, and then the calculated value of CSS direction value, based on the actual text in the node, is used to choose a CSS class name that in the rtl case will animate to the left at first, instead of the right.

There is also a change to align short ltr email addresses in an rtl UI will show up left aligned instead of right aligned, and rtl email addresses in ltr UI will be right aligned, to match the layout of the UI.

Tested on device with both an ltr email and one made up of rtl characters, trying a short and long email address in both cases.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8600071 [details] [review]
[gaia] jrburke:bug1159982-email-addr-scroll-dir > mozilla-b2g:master

It all sounds very plausible.

Comment 6

[GitHub Autolander Bot]

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

Comment 7

[James Burke [:jrburke]]

Comment on attachment 8600071 [details] [review]
[gaia] jrburke:bug1159982-email-addr-scroll-dir > mozilla-b2g:master

[Approval Request Comment]
Asking for 2.2 uplift since we have a fix. While not the most important RTL fix to have, it would be nice to round out the support for it.

[Bug caused by] (feature/regressing bug #):
Part of RTL support.

[User impact] if declined:
The email address will not be displayed correctly when inspecting it from a person bubble.

[Testing completed]:
On device, using combination of ltr UI with rtl text, rtl UI with ltr text, with long and short strings.

[Risk to taking this patch] (and alternatives if risky):
Low. Scope limited to the only place where we use the marquee module. Most of the code is declarative in the CSS.

[String changes made]:
none

Comment 8

[bhavana bajaj [:bajaj]]

Comment on attachment 8600071 [details] [review]
[gaia] jrburke:bug1159982-email-addr-scroll-dir > mozilla-b2g:master

Given RTL is a new feature in 2.2, this low risk patch is good to go.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Sue]

Attachment: verify_video.mp4

This issue has been verified passed on latest build of Flame 2.2/3.0 and Nexus 5 2.2/3.0 with the same steps in comment 0.
Result: The full email address is shown correctly by the animation.
See attachment:verify_video.mp4
Rate:0/3

Device: Flame 2.2 (pass)
Build ID               20150504002502
Gaia Revision          8d14361337e608c8cdf165ea5034db5eda23b618
Gaia Date              2015-05-01 18:23:46
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cb7cb6597c91
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150504.041436
Firmware Date          Mon May  4 04:14:48 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (pass)
Build ID               20150504160201
Gaia Revision          70077825aab2c7a79611befb40a5fe7e610d5443
Gaia Date              2015-05-04 18:09:33
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/102d0e9aa9e1
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150504.193019
Firmware Date          Mon May  4 19:30:30 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150504002502
Gaia Revision          8d14361337e608c8cdf165ea5034db5eda23b618
Gaia Date              2015-05-01 18:23:46
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cb7cb6597c91
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150504.041256
Firmware Date          Mon May  4 04:13:14 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (pass)
Build ID               20150504160201
Gaia Revision          70077825aab2c7a79611befb40a5fe7e610d5443
Gaia Date              2015-05-04 18:09:33
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/102d0e9aa9e1
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150504.193136
Firmware Date          Mon May  4 19:31:54 EDT 2015
Bootloader             HHZ12f

Comment 11

[Sue]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/16258/