[RTL] SMS notification for dual sim puts wrong parenthesis on wrong side of the sentence

VERIFIED FIXED in 2.2 S7 (6mar)

Status

defect
P2
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: pcheng, Assigned: gmarty)

Tracking

unspecified
2.2 S7 (6mar)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [3.0-Daily-Testing][systemsfe])

Attachments

(5 attachments)

Posted image screenshot of bug
Description:
SMS Notification for dual-sim environment puts which SIM a message is received from in parenthesis. And in RTL environment, the beginning of the parenthesis is being put to the end and it's using the wrong round bracket.

Prerequisite:
DUT is using dual sims

STR:
(optional) Create contact A in Contacts app
1) Have contact A send an SMS to DUT
2) Observe the SMS notification on DUT

Expected: It should say:
(SIM 1) Name
Message content

Actual: It says:
SIM 1) Name)
Message content

See screenshot for bug behavior.
This issue occurs on Flame 3.0 Master and on Flame 2.2.

Device: Flame 3.0
BuildID: 20150218010226
Gaia: 82f286f10a41aab84a0796c89fbefe67b179994b
Gecko: 9696d1c4b3ba
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

Device: Flame 2.2
BuildID: 20150218002515
Gaia: da509caa7395d3d090ce973e8de082b4680a590d
Gecko: 96da179a7d3a
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
QA Whiteboard: [rtl-impact][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [3.0-Daily-Testing]
QA Whiteboard: [rtl-impact][QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage+]
Flags: needinfo?(ktucker)

Comment 3

4 years ago
Flagging Francisco to see if we can get anyone assigned to this before Monday. P2.
Flags: needinfo?(francisco)
Priority: -- → P2
Hi Stephany, this is not SMS specific, is happening in the window management component.

You'll need more info from Gregor.
Flags: needinfo?(francisco) → needinfo?(anygregor)
Flags: needinfo?(anygregor)
Whiteboard: [3.0-Daily-Testing] → [3.0-Daily-Testing][systemsfe]
Component: Gaia::System::Window Mgmt → Gaia::System
blocking-b2g: --- → 2.2?
blocking-b2g: 2.2? → 2.2+
I'll take a look.
Assignee: nobody → mhenretty
Actually, let's have Guillaume take a look.
Assignee: mhenretty → gmarty
I found that this similar issue also occurs when selecting a language in Browser. I'm not sure if these two are of the same cause, but here is the STR from Browser:

1) Navigate to http://news.google.com
2) Scroll to the bottom of the page and tap on dropdown list of 'Select edition'
3) Scroll to the bottom of the value selection page

See attached screenshot for bug behavior.

Please let me know if a separate bug needs to be filed.
Assignee

Comment 9

4 years ago
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

Julien, can you review this RTL related patch?
Attachment #8568574 - Flags: review?(felash)
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

As discussed on IRC, we miss the fix for the notification part.

Also I think the patch is not correct, we should add dir="auto" on each <li> otherwise I fear of incorrect behavior in some cases.
Attachment #8568574 - Flags: review?(felash)
Changing title to make this a global issue instead of just one single area.
Summary: [RTL] SMS notification for dual sim puts wrong parenthesis on wrong side of the sentence → [RTL] Wrong parenthesis is being used on multiple areas
Pi Wei, I don't think this is a good idea.

The global issue is related in how Unicode works, and we need to adjust the markup or the CSS in all places where this can happen. We won't fix it all in one patch.
(The parenthesis issue should eventually be fixed in bug 922963, but not the ".S.U" one).

I think it's better to file separate bug for different areas.

We _could_ fix the 2 cases you reported here, but please don't dupe other bugs.
We'll hold off on filing separate bugs since we're unsure what the scope of bug 922963 will be fixing and we want to avoid dupes. Changing title back.
Summary: [RTL] Wrong parenthesis is being used on multiple areas → [RTL] SMS notification for dual sim puts wrong parenthesis on wrong side of the sentence
TBH I don't see bug 922963 moving soon. So don't refrain yourself ;)
Assignee

Comment 16

4 years ago
Comment on attachment 8569936 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options-Notification > mozilla-b2g:master

I'm still unsure whether I should open a new bug or not, so I created a different patch for the notification issue.
Etienne, can you review it?
Attachment #8569936 - Flags: review?(etienne)
Assignee

Comment 17

4 years ago
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

Julien, I applied the changes we discussed offline. How does it look now?
Attachment #8568574 - Flags: review?(felash)
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

look at http://everlong.org/mozilla/test-select/
It works fine in RTL :) (I think it was not fine with your first patch)

But in LTR the "arabic" option is aligned right, I think it should be aligned left. We need a "text-align: left" somewhere :)
Attachment #8568574 - Flags: review?(felash)
Assignee

Comment 19

4 years ago
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

Patch updated!
Attachment #8568574 - Flags: review?(felash)
Comment on attachment 8569936 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options-Notification > mozilla-b2g:master

r=me with a small unit test in notifications_test.js
Attachment #8569936 - Flags: review?(etienne) → review+
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

lgtm, let's see what the peers think :)
Attachment #8568574 - Flags: review?(pivanov)
Attachment #8568574 - Flags: review?(felash)
Attachment #8568574 - Flags: review?(etienne)
Attachment #8568574 - Flags: feedback+
Attachment #8568574 - Flags: review?(pivanov) → review+
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

stamp (looks like it already landed :))
Attachment #8568574 - Flags: review?(etienne) → review+
Assignee

Comment 24

4 years ago
Comment on attachment 8568574 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): RTL
[User impact] if declined: The parentheses are mismatched in the select tags in web pages
[Testing completed]: Manual testing
[Risk to taking this patch] (and alternatives if risky): Low as it's only markup and CSS change
[String changes made]: None
Attachment #8568574 - Flags: approval-gaia-v2.2?(bbajaj)
Assignee

Comment 25

4 years ago
Comment on attachment 8569936 [details] [review]
[gaia] gmarty:Bug-1134453-RTL-value-selector-options-Notification > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): RTL
[User impact] if declined: The parentheses are mismatched in notifications toaster and in the utility tray
[Testing completed]: Manual testing
[Risk to taking this patch] (and alternatives if risky): Low as it's only markup and CSS change
[String changes made]: None
Attachment #8569936 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8568574 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Attachment #8569936 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
The issues on comment 0 and comment 7 have been verified fixed on Flame 3.0 master,

Parentheses now display as expected in Arabic in those cases.

Device: Flame 3.0 Master (full flash 319MB mem)
BuildID: 20150303010233
Gaia: c8ed1085a67490a1ecd7f275e5de9487e1b93b1d
Gecko: 0b3c520002ad
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Device: Flame 2.2 (full flash 319MB mem)
BuildID: 20150303002527
Gaia: 3d188c414e30acc392253d5389a42352fcfbc183
Gecko: c89aad487aa5
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (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: [rtl-impact][QAnalyst-Triage+] → [rtl-impact][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Issue is also verified fixed on Flame 2.2 (missed this sentence on comment 27)
QA Whiteboard: [rtl-impact][QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.