Closed Bug 1099449 Opened 5 years ago Closed 5 years ago

[email] rtl: properly display message bodies

Categories

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

defect
Not set

Tracking

(b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: jrburke, Assigned: jrburke)

References

Details

Attachments

(1 file)

With RTL, it causes some wrinkles with our scaling in iframe_shims for message bodies. Some background in bug 1064617 comment 6, but some constraints:

* message body should show left or right aligned based on the body text, not UI text direction.
* this affects transform origins, scaling.
* Also may affect what direction the screen can be scrolled horizontally.

Until this is resolved, I expect that viewing LTR messages in RTL UI or vice versa to have problems.
Also, be sure that click targets continue to work.
feature-b2g: --- → 2.2?
Attached file GitHub pull request
This turned out to not be so bad once I understood what was going on. I also made this much worse in a previous experimental changeset where I was trying to set the dir on a container element for the iframe. That experiment made this seem really messed up.

Fortunately for baseline RTL support that was landed in bug 1064617 comment 11, I had left that misguided change out, and that changeset just had the fix to properly show the body contents with the zoom.

The remaining issue is the tap calculations. The zooming works fine already. So this changeset just fixes the tap calculations.

I tested the following combos, where the first value is the dir of the email app UI and the second was what the HTML body content was:

RTL -> RTL
LTR -> RTL
RTL -> LTR
LTR -> LTR 

The main thing that I was over-analyzing is that we just leave the iframe doc in the default mode of LTR, and let the email body content set its direction by itself, and the other stuff sorts itself out, as long as we adjust for the scroll value reported in the email app UI RTL case.

:asuth, I tested an RTL email by using an Al Jazeera email list signup confirmation, if you test with Jean's account you can see it. I am hoping that was a good test case for viewing an RTL email. It seemed to be based on display output.
Assignee: nobody → jrburke
Status: NEW → ASSIGNED
Attachment #8547871 - Flags: review?(bugmail)
Comment on attachment 8547871 [details] [review]
GitHub pull request

Ha, that is not what I would have expected from scrollLeft!  The red square thing seems like a fantastic idea, so I'm reviewing this primarily by inspection and the understanding you used that and this seemed to work.  And that at the very least this won't make things work any worse since it's predicated on RTL being active anyways.  (And there's been an obvious large amount of QA work and such investigating these things, so I'll leave it to them.)
Attachment #8547871 - Flags: review?(bugmail) → review+
Merged in master:
https://github.com/mozilla-b2g/gaia/commit/1af31389d35088cc9fe6720d8b22a5e9af50b574

from pull request:
https://github.com/mozilla-b2g/gaia/pull/27335
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Keywords: verifyme
Comment on attachment 8547871 [details] [review]
GitHub pull request

Forgot that this missed the 2.2 branch cut, so asking for uplift for that branch as it is part of the baseline RTL support in email:

[Approval Request Comment]

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

[User impact] if declined:
Tapping on email links in HTML emails will not work if the UI is in RTL direction.

[Testing completed]:
Tested on flame device on master

[Risk to taking this patch] (and alternatives if risky):
Very low. Only affects RTL UX, and without it, tapping on links is broken.

[String changes made]:
None
Attachment #8547871 - Flags: approval-gaia-v2.2?
feature-b2g: 2.2? → ---
Attachment #8547871 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
This issue is verified fixed on Flame 3.0 and 2.2

The bodies of messages in the email app will display based on the language they are written in (RTL or LTR).

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150205010209
Gaia: 2b83a6d5d1185a438b5bbd287497ac2743b501db
Gecko: 34a66aaaca81
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0


Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150205002503
Gaia: c2047a46e29696238e9b4c9caaba47736421449a
Gecko: adfba0a07e9b
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
Flags: needinfo?(pbylenga)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(pbylenga)
You need to log in before you can comment on or make changes to this bug.