Closed Bug 1058743 Opened 10 years ago Closed 10 years ago

[RTL] Dialer should start out as in RTL right away

Categories

(Firefox OS Graveyard :: Gaia::Dialer, defect, P1)

Product:
Firefox OS Graveyard
Component:
Gaia::Dialer
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

(blocking-b2g:2.2+, b2g-v2.1 wontfix, b2g-v2.2 verified, b2g-master verified)

Status:
VERIFIED FIXED
Milestone:
2.2 S9 (3apr)
Project Flags:
blocking-b2g 2.2+
Tracking Flags:
Tracking Status
b2g-v2.1 --- wontfix
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: jlorenzo, Assigned: gsvelto)

References

Details

(Whiteboard: [2.1-Arabic-RTL-bug-bash][planned-sprint c=3])

Attachments

(4 files)

Switch to RTL mode on startup - video
4.04 MB, video/3gpp
Details
[gaia] gabrielesvelto:bug-1058743-do-not-lazy-load-code-and-styles > mozilla-b2g:master
46 bytes, text/x-github-pull-request
drs
: review+
drs
: feedback+
bajaj
: approval-gaia-v2.2+
Details | Review
video_v3.0.MP4
1.56 MB, video/mp4
Details
Verify2_Flame2.2_Pass.mp4
1.39 MB, video/mp4
Details
Build Information Gaia 4d1d0ea5a82cddeeab497774cfa1703639e3c7d9 Gecko https://hg.mozilla.org/mozilla-central/rev/dc352a7bf234 BuildID 20140826040204 Version 34.0a1 ro.build.version.incremental=110 ro.build.date=Fri Jun 27 15:57:58 CST 2014 B1TC00011230 Steps to Reproduce: 0. Close the dialer app. 1. Open the dialer app Expected Results: The app starts directly on RTL mode. Actual Results: You see the LTR mode for about half a second, then it switches. See attached video Repro rate: 100%
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Mass Edit: adding the [rtl-meta]
Whiteboard: [2.1-Arabic-RTL-bug-bash] → [rtl-meta]
Whiteboard: [rtl-meta] → [rtl-meta][2.1-Arabic-RTL-bug-bash]
QA Whiteboard: [rtl-impact]
Whiteboard: [rtl-meta][2.1-Arabic-RTL-bug-bash] → [2.1-Arabic-RTL-bug-bash]
Flags: in-moztrap+
Reopening. Issue still occurs after bug 1037603 was verified fixed. Environmental Variables: Device: Flame 3.0 (319MB)(Full Flash) Build ID: 20150319010201 Gaia: c39e15f631de80c69467fda0d4ea0bcda9e194ca Gecko: cf1060d8ce9f Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 39.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0
Status: RESOLVED → REOPENED
QA Whiteboard: [rtl-impact] → [QAnalyst-Triage?][rtl-impact]
status-b2g-v2.2: --- → affected
status-b2g-master: --- → affected
Flags: needinfo?(pbylenga)
Resolution: DUPLICATE → ---
Flagging gandalf to see if he can help with this one. thanks
Flags: needinfo?(gandalf)
Wait - dialer isn't supposed to show up in RTL mode. It should stay same as in LTR as per UX specs. I'm not seeing the initial issue described in this bug. However, I do see the icons below being swapped, which from what I understand they shouldn't be since not indicating direction. Can you please confirm what you're seeing Martin?
Flags: needinfo?(gandalf) → needinfo?(mshuman)
Priority: -- → P2
Ok unless I'm mistaken: the actual issue here is that the "Call" icon and "add contacts" icon under dialer start out as LTR briefly and then get swapped. This should not happen. From what I understand, these icons should remain in the same position as LTR and *not* be swapped at all. Changing the name of the bug to reflect this. Also, nominating
blocking-b2g: --- → 2.2?
Priority: P2 → P1
Summary: [RTL] Dialer does not start directly in RTL mode → [RTL] Dialer should start out as in LTR right away
Comms triage: Reproduced the issue on master. It takes about 2 seconds to see the swap. This is hurting the user experience on a critical feature. Blocking
blocking-b2g: 2.2? → 2.2+
Actually I'm not sure if this is a problem related to dialer, but we should start with <html dir="rtl"> seems that we don't start with that attribute in the html node and later on we get it. This is a guess. In that case is not a dialer problem, but a platform one.
I confirm the issue I am seeing is what is described in Comment 6. Numpad icons are not mirroring, but the Call and Add Contact icons are mirroring shortly after launch.
Flags: needinfo?(mshuman)
Whiteboard: [2.1-Arabic-RTL-bug-bash] → [2.1-Arabic-RTL-bug-bash][planned-sprint c=?]
Target Milestone: --- → 2.2 S9 (3apr)
Assignee: nobody → gsvelto
Summary: [RTL] Dialer should start out as in LTR right away → [RTL] Dialer should start out as in RTL right away
Whiteboard: [2.1-Arabic-RTL-bug-bash][planned-sprint c=?] → [2.1-Arabic-RTL-bug-bash][planned-sprint c=3]
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(pbylenga)
It looks like there's two facets to this issue: - At startup we're lazy-loading CSS code and a bunch of other stuff. This is not directly causing the issue but it's making it more apparent as it introduces a significant delay. We shouldn't be lazy-loading stuff we need right away as it just slows things down. - We're lazy-loading l10n.js and this is what is actually causing the switch to happen late. I'm working on removing LazyL10n as it does not saving us much and it's causing this problem.
This is a WIP PR that solves the issue at hand. The issue is directly caused by us lazy-loading the l10n sources which prevent the proper layout from working right away and aggravated by the fact we're lazy-loading plenty of stuff that is actually needed right away. The patch-set is a WIP because it breaks tests left and right and I still need to fix them up. I've done some light performance testing and the results look good for now. Visible time is unaffected as all the sources are marked with defer and overall load time is lower though I haven't measured precisely by how much yet. It's in the order of 100-200ms from what I can tell.
Comment on attachment 8583071 [details] [review] [gaia] gabrielesvelto:bug-1058743-do-not-lazy-load-code-and-styles > mozilla-b2g:master I've cleaned up the tests and did some manual testing on the phone and it's looking good. I'm not asking for review just yet because I want to switch using NotificationHelper as suggested by :zbraniecki on the PR. I also want to post some test results for the performance aspect of this change. The rest should be in good shape.
Attachment #8583071 - Flags: feedback?(drs)
Comment on attachment 8583071 [details] [review] [gaia] gabrielesvelto:bug-1058743-do-not-lazy-load-code-and-styles > mozilla-b2g:master Looks good. As discussed offline, we should try eagerly loading l10n.js and see if that improves anything.
Attachment #8583071 - Flags: feedback?(drs) → feedback+
Comment on attachment 8583071 [details] [review] [gaia] gabrielesvelto:bug-1058743-do-not-lazy-load-code-and-styles > mozilla-b2g:master This is ready for review with some significant changes. After more in-depth testing I've decided to remove the first patch which got rid of the lazy loading. I measured almost 350ms worth of savings in the overall startup time with that patch but it was prone to causing the infamous flash-of-white when the application started. This was most likely due to some JS code touching the DOM before the first reflow had finished thus postponing it and displaying the app as an empty white rectangle in the meantime. Since I didn't have enough time to investigate what exactly was causing the issue and I could reproduce often enough (it's timing dependent) I've decided to cut down the patch to only removing the lazy-loading of the l10n sources. I've experimented with removing the defer tag from them but I've noticed that could also trigger the flash-of-white so I've abandoned that approach too. This patch fixes the problem in this bug without trying to improve things further; we'll do that in a follow up when we have less pressing time constraints. I've also used the NotificationHelper class to get rid of the mozL10n.get invocations we used and adjusted the unit-tests accordingly.
Attachment #8583071 - Flags: review?(drs)
Comment on attachment 8583071 [details] [review] [gaia] gabrielesvelto:bug-1058743-do-not-lazy-load-code-and-styles > mozilla-b2g:master Looks good. I left a couple of comments on the PR, including in the "conversation" tab, since they addressed code that aren't near the changes.
Attachment #8583071 - Flags: review?(drs) → review+
Addressed the comments and refreshed the PR, ready to land.
Keywords: checkin-needed
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment on attachment 8583071 [details] [review] [gaia] gabrielesvelto:bug-1058743-do-not-lazy-load-code-and-styles > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Using the dialer in RTL mode [User impact] if declined: The dialer appears in LTR mode then switches to RTL after a couple of seconds providing a fairly poor user experience [Testing completed]: Tested on a device (Flame) and on the emulator [Risk to taking this patch] (and alternatives if risky): This patch touches the l10n code so it might be worth keeping our eyes open for regressions in that area even though I have manually tested the affected code paths [String changes made]: None
Attachment #8583071 - Flags: approval-gaia-v2.2?(bbajaj)
Attached video video_v3.0.MP4 —
This issue has been verified successfully on latest build of Flame 3.0 with the same steps in comment 0. When user launch Phone app, the app starts directly on RTL mode. See attachment:video_v3.0.MP4 Rate:0/5 Device: Flame 3.0 (pass) Build ID 20150329010203 Gaia Revision 67ad91f3f660b1f16b354ee4c5159ddc5a74d149 Gaia Date 2015-03-28 10:02:40 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/385840329d91 Gecko Version 39.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150329.042104 Firmware Date Sun Mar 29 04:21:16 EDT 2015 Bootloader L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage+][rtl-impact][MGSEI-Triage+]
status-b2g-master: affected → verified
Attachment #8583071 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
This issue has been verified pass on latest build of Flame 2.2 with the same steps in comment 0. See attachment:Verify2_Flame2.2_Pass.mp4 Reproducing Rate:0/10 Flame 2.2 build (Unaffected): Build ID 20150401002624 Gaia Revision 8b3086ad3963f1707e2bee9094baccafffe161c4 Gaia Date 2015-03-31 21:48:06 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/20b67213a047 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150401.042225 Firmware Date Wed Apr 1 04:22:36 EDT 2015 Bootloader L1TC000118D0
Hi William, As I known, Flame 2.1 does not support RTL, shall we change "status-b2g-v2.1" as "wontfix" and close this issue? Or could you please have someone to help with this? Thanks!
Flags: needinfo?(whsu)
Yes, that's a good way to express the lack of RTL support in 2.1. Thanks for proposing it, Sue.
Status: RESOLVED → VERIFIED
status-b2g-v2.1: affected → wontfix
Flags: needinfo?(whsu)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: