Status

defect
RESOLVED WORKSFORME
5 years ago
3 years ago

People

(Reporter: swilkes, Unassigned)

Tracking

({meta})

unspecified
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(ux-b2g:2.2)

Details

(Whiteboard: [rtl-meta])

Attachments

(2 attachments)

This is the meta bug for RTL support in the Email app. The release target for full Gaia (all apps) RTL support is 2.2.
Depends on: 1064640
Depends on: 1064643
Duplicate of this bug: 994053
Depends on: 1087051
Depends on: 1087055
Alias: email-rtl
Depends on: 1087066
Depends on: 1087085
Some logistical notes for dev, applies to all rtl bugs, so commenting about it here:

1) There is a good thread on dev-gaia about some mechanics involved in doing the changes:

https://groups.google.com/forum/#!topic/mozilla.dev.gaia/GzIIlTUKvAg

2) I am hoping we can do a bulk change for email for at least this initial set of rtl bugs, and after we land bug 1005446, since that one changes a bunch of files. I am not marking it as a hard dependency, since if we do get pull requests for these bugs before it lands, I would want to take them, and I handle merging them into that other pull request. I am hopeful we can land that bug this week, it is at the point of formal review.
QA Contact: edchen
Depends on: 1058693
No longer depends on: 1058693
I have a first pass at the RTL support for email. You can install this app zip via the web IDE (unzip it by double clicking in your file system first):

http://jrburke.com/work/gaia/email-rtl-1.zip

Some notes: I suggest setting up the email account, particularly if it is a gmail account, before switching the phone's locale to the fake "Mirrored English" one. I think our weird, fictional locale gives some servers some trouble.

So it is best if you set up the account and fetch the emails you want to look at before switching the locale. I don't know if it was the locale thing in particular or other phone problems, but I had some trouble going back and forth in accounts, and then getting network operations to the email servers to work. Restarting the phone, in a real locale seemed to help. I do not expect these problem with a real RTL locale.

You can enable the fake locales by going to Settings, Developer, and check the "Psuedo-localization" checkbox. Then you can go to Settings, Language, and select "Mirrored English" which gives an approximation of an RTL language.

Some notes on what is implemented so far:

* Even though the locale is set to "Mirrored English", text input seems to be normal English, so you do not get the full feel of text entry. I would like to have a native Arabic speaker/reader try to input text to confirm we have the inputs wired up correctly. I believe I do (try typing an @ sign in some of the fields, and you can see it jump to the left), but there could be some tweaks needed.

* Similar for in search: when we show matches, we color highlight the match in the search results. It would be good to know from a person who uses a real RTL language that it looks OK, not something we can simulate with the fake locale hack.

* I believe I still have to fix some issues with zooming in the message reader, so if you see weirdness there, it is a known issue.

* I left the chevrons in the settings screens, as they seemed to light up well with other form/select box controls that end up on the left. 

The attachment for this comment shows what that looks like. However, I know this was a tricky UX question. I can remove them if you like, but just trying out an option so that you can see it in practice, and in relation to the other form fields. It is easy to remove or even right-align if you want to try those things.

Once we work out the final story for the chevrons, then I think this baseline implementation is good enough that I would like to land it, then focus on filing bugs for specific cleanup issues.

However, I will wait for ux-review+ and need-info OK from product before asking review on this first round of changes. The dev work is in this branch:

https://github.com/jrburke/gaia/compare/bug1064617-email-rtl
Flags: needinfo?(swilkes)
Attachment #8522580 - Flags: ui-review?(jhuang)
Assignee: nobody → jrburke
Target Milestone: --- → 2.1 S9 (21Nov)
Comment on attachment 8522580 [details]
Chevrons left aligned in email account settings

Hi James, nice work!!!!
I just tried the patch u provided.
Apart from the issues you mentioned on last comment, it looks overall great!
As far as I know, framework team has started to come out a new idea for chevron.
So we can wait for their updates.
Attachment #8522580 - Flags: ui-review?(jhuang) → ui-review+
I did an update to the zip, I believe I did some things incorrectly in the last one:

* used an attribute selector instead of a psuedo selector (as an end user you would not have seen a difference vs. the first zip).
* I removed dir="auto" from user input form elements, where I needed to make sure they all had it, as well as adding it to some of the display areas where the text came from user input.

I updated those items, and this is the latest zip:

http://jrburke.com/work/gaia/email-rtl-2.zip

You should not notice much difference from the first zip, except that now:

* text ellipsis truncation is correct for LTR text viewed in a RTL language used for the UI.
* text input starts off on the left now instead of the right, but not sure if that is because I only have a US keyboard, need a native speaker with correct keyboard to try it. But it should fix the type of issue noted in bug 1064643, if using LTR input.
Aaannnd... version 3:

http://jrburke.com/work/gaia/email-rtl-3.zip

I was aggressive with the dir="auto" and applied it to the rootBodyNode in message_reader, so that the user intent for the message body was reflected, but this causes havoc with the iframe_shims calculations. So for now I removed dir="auto" and expect to have a fix later for all the iframe_shims related issues, including image loading in a separate bug, which I will attach as a blocking bug for this bug.

I expect the dir="auto" is the correct thing for the user intent, we just need to work out some other issues with HTML body display first, and I want to do that as targeted fixes after landing this first pass.
Depends on: 1099449
Posted file GitHub pull request
Going to go ahead and start review on this so we can land the first pass at this. 

By landing this changeset, it does not mean this bug is "fixed", but rather we can start opening targeted bugs to fix the more isolated issues, like bug 1099449, instead of the more global issues.
Attachment #8523293 - Flags: review?(bugmail)
Sorry - clearing the ni? for me since Juwei has this. Overall looks great except for the targeted issues you already mention. THANK YOU! Very nice work.
Flags: needinfo?(swilkes)
feature-b2g: --- → 2.2+
Target Milestone: 2.1 S9 (21Nov) → 2.2 S1 (5dec)
Comment on attachment 8523293 [details] [review]
GitHub pull request

r=asuth, ideally with the click logic for RTL fixed in this patch.  Otherwise please spin-off a bug for that (which maybe gets resolved by your html display experiments anyways?).
Attachment #8523293 - Flags: review?(bugmail) → review+
There definitely still needs to be some email body cleanup work, tracked in bug 1099449. The hope is to land this bigger baseline support patch, but not close this bug until we get all follow-on bugs fixed up.
Baseline support changeset in Gaia master:
https://github.com/mozilla-b2g/gaia/commit/28a8edaafa3a73c7fc837bd0b543681cb46ed15d

from pull request:
https://github.com/mozilla-b2g/gaia/pull/26162

Still keeping this open since a meta bug, and will get to scrubbing dependent bugs as the next step for this bug.
[Tracking Requested - why for this release]:
feature-b2g: 2.2+ → ---
tracking-b2g: --- → ?
Target Milestone: 2.2 S1 (5dec) → ---
Hi James - Let me know how close the full patch is. Bhavana are evaluating for 2.2 inclusion and timing, not for "full RTL support in 2.2" but for incremental RTL improvements.
Flags: needinfo?(jrburke)
Stephany: I just scrubbed through the dependent bugs on this ticket. I did a fix for bug 1087066, and I believe the bug about direction in inputs needs a real RTL keyboard to test the full fix. So that leaves these two bugs that I know need work:

* Bug 1099449 - [email] rtl: properly display message bodies: This is for HTML email, plain text emails should be fine.
* Bug 1087051 - Cannot view image in email when language is set to Arabic: This one may just fall out of the fix that is done for bug 1099449, but if not, will likely be related. Keeping it open for now as a separate bug.

So the big missing piece is proper functioning of HTML body display in RTL. I would also not be surprised if we have some smaller nit fixes once we have real RTL users doing some testing with a real RTL keyboard installed.

The changes done for this ticket so far should be safe to carry in 2.2, should not affect LTR users, and I would not want to back them out for 2.2. 

I would like to just get to fixing the above HTML body bugs in the 2.2 timeframe, just so we can mentally feel this is ready for real user testing, but may not make it depending on holiday schedules and if other things in 2.2 get scheduled that steal time.
Flags: needinfo?(jrburke)
Adding whiteboard tag [rtl-impact]
Whiteboard: [rtl-impact]
Mass Edit: adding the [rtl-meta]
Whiteboard: [rtl-impact] → [rtl-meta]
Depends on: 1118062
Depends on: 1118893
Depends on: 1119943
Assignee: jrburke → nobody
Depends on: 1122450
Depends on: 1125743
Depends on: 1129220
Depends on: 1131485
Depends on: 1141000
Depends on: 1144814
Depends on: 1152220
Depends on: 1159982
Depends on: 1165205
Depends on: 1177350
Depends on: 1177351
Depends on: 1178207
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.