[Dialer] "Withheld number" string is missing on call log

VERIFIED FIXED in 2.2 S9 (3apr)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: ychung, Assigned: steveck)

Tracking

({regression})

unspecified
2.2 S9 (3apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

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

Attachments

(3 attachments)

Reporter

Description

4 years ago
Description:
On the call log screen, there is no "Withheld number" string for an incoming or missed call from a hidden number.

Repro Steps:
1) Update a Flame to 20150323010204.
2) Receive an incoming call from a hidden number.
3) End the phone call.
4) Open Phone app, and select the call log screen.
5) Observe the record for the incoming call from the hidden number.

Actual:
No text or number is shown.

Expected:
"Withheld number" string is displayed.

Environmental Variables:
Device: Flame Master (KK, 319mb, full flash)
Build ID: 20150323010204
Gaia: 9b6f3024e4d0e62dd057231f4b14abe1782932ab
Gecko: e730012260a4
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

Repro frequency: 10/10
See attached: video clip, logcat
https://youtu.be/9ReVbN3BoO4
Reporter

Comment 1

4 years ago
This issue also reproduces on Flame 2.1.

Result: No text or number is shown.

Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150323002504
Gaia: 7f367fc98ffdd183f21d2cdfe20556ab877ece34
Gecko: 3ea0eaeda353
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

-----------------------------
This issue does NOT reproduce on Flame 2.1.

Result: "Withheld number" string is displayed.

Device: Flame 2.1  (KK, 319mb, full flash)
Build ID: 20150323001203
Gaia: 13c85d57f49b4bfd657ff674f2b530c141c94803
Gecko: aeabf85a622c
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Reporter

Comment 2

4 years ago
Posted image NoString.png
Regression from 2.1 so nominating 2.2? 

Let's get a regression window.
blocking-b2g: --- → 2.2?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Reporter

Updated

4 years ago
QA Contact: ychung
triage: regression
blocking-b2g: 2.2? → 2.2+
Reporter

Comment 5

4 years ago
B2G-inbound Regression Window:

Last Working Environmental Variables:
Device: Flame 3.0
BuildID: 20150225095511
Gaia: 05e6a5e4f4f1e27197f798144a177c1b421eadd0
Gecko: 3fa128eeb95a
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

First Broken Environmental Variables:
Device: Flame 3.0
BuildID: 20150225105537
Gaia: c98bc323911be609bc3bace8aafb10deca020111
Gecko: a847c6c4dfc6
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Last Working Gaia First Broken Gecko: Issue does NOT reproduce 
Gaia: 05e6a5e4f4f1e27197f798144a177c1b421eadd0
Gecko: a847c6c4dfc6

First Broken Gaia Last Working Gecko: Issue DOES reproduce
Gaia:  c98bc323911be609bc3bace8aafb10deca020111
Gecko: 3fa128eeb95a

https://github.com/mozilla-b2g/gaia/compare/05e6a5e4f4f1e27197f798144a177c1b421eadd0...c98bc323911be609bc3bace8aafb10deca020111

possibly caused by bug 1131490
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Contact: ychung
Augustin, can you take a look at this please? This might have been caused by the work done on 1131490.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(augustin.trancart)
Assignee

Comment 7

4 years ago
Hi Augustin, I can take a look first if you have no time on this right now.
Assignee

Comment 9

4 years ago
Comment on attachment 8582932 [details] [review]
[gaia] steveck-chung:dialer-withheld-number > mozilla-b2g:master

Hey Gabriele, it's a simple patch that address some issue in bug 1131490, which introduced bdi element to the contact/number field in the call log list. But the the withheld number l10n key is still set to the container, and l10n will return error if target element contains other element(bdi in this case) inside.

I set the key to bdi element instead in order to keep the structure as same as other cases. Another idea is don't append the bdi element if number is withheld. It's up to you :)
Attachment #8582932 - Flags: feedback?(gsvelto)
Assignee

Updated

4 years ago
Assignee: nobody → schung
Hi Steve! Thanks for the fix!

It sounds odd to me that this reproduces in 2.1. My patch is not on that version, what do you think?

By the way, at Phoxygen for 2.0m, we now use dir="auto" instead of bdi to avoid this kind of regression (see http://codepen.io/fabi1cazenave/details/emjjJL/) (credits to Kaze for this one).
Pro : html markup stay the same, only CSS changes. Thus, here I would have only needed to touch the js to set the dir="auto", which is harmless...
Cons: you can't use start/end on this element any more, you have to manually set left and right if you need alignment, margin, paddings, borders etc.
Here, if I'd have done that in the first place, everything would have been all right :-) This covers nearly all the mixed content issues we have, including '+' misplacement and ellipsis.

In this codepen, the alignment is done at the top of the file, but in real situation, we prefer to do things this way

> .mySelector {
>    /* every rule that are not directional */
>    background-color: #ffff;
>    display: inline-block;
> }
> /* right after .mySelector */
> html[dir="ltr"} .mySelector {
>   text-align: left;
>   float: left; 
>   /* and every other directional LTR rules for this selector  */
> }
>
> html[dir="rtl"] .mySelector {
>   text-align: right;
>   float: right;
>   /* and every other directional RTL rules for this selector */
> }

We found it has several advantages : 
- less error-prone than grouping all the RTL rules at the end of the file, far away from their LTR counterparts. More comfortable for the developer and the reviewer.
- RTL is now a first class citizen, devs will think about it. In the current situation, RTL will get broken again and again, because neither devs nor reviewers will remember there is a RTL rules at the end of the file (I can't blame them :-) )
- in a more subtle way, it ensures that LTR and RTL rules have the same specificity and priority. (bugs because of these differences are rare, but difficult to debug)

I guess it is too late for this patch, but we found that the gain (both in term of comfort and quality) far outweighs the little increase of verbosity.
Flags: needinfo?(augustin.trancart)
Comment on attachment 8582932 [details] [review]
[gaia] steveck-chung:dialer-withheld-number > mozilla-b2g:master

LGTM; I feel silly that I've overlooked this when I reviewed the original change.
Attachment #8582932 - Flags: feedback?(gsvelto) → feedback+
Assignee

Comment 12

4 years ago
Hi Augustin, it's not reproducible in 2.1(see comment 1), don't worry :)

I would prefer the keep the original markup if possible, but since we are still using start/end in dialer, applying dir=auto seems not a safe option for now. And there might be another possible side effect that dir=auto might not set the direction by content if parent set css direction property already. You might free from this issue if you don't touch any css property for direction issue, it's just a reminder.

So in this case, I will still propose this fix for now. But we can still revisit all the direction issue in the future for sure.
Assignee

Comment 13

4 years ago
Comment on attachment 8582932 [details] [review]
[gaia] steveck-chung:dialer-withheld-number > mozilla-b2g:master

Patch updated with a withheld number test for this change.
Attachment #8582932 - Flags: review?(gsvelto)
Comment on attachment 8582932 [details] [review]
[gaia] steveck-chung:dialer-withheld-number > mozilla-b2g:master

Looks good to me, thanks for having added the test too.
Attachment #8582932 - Flags: review?(gsvelto) → review+
Assignee

Updated

4 years ago
Keywords: checkin-needed
http://docs.taskcluster.net/tools/task-graph-inspector/#8GHcB7tHQg6RT_PWp6xlgQ

The pull request failed to pass integration tests. It could not be landed, please try again.
Assignee

Updated

4 years ago
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Since it is a 2.2 blocker, could you please uplift to 2.2? Thanks!
Flags: needinfo?(schung)
Assignee

Comment 18

4 years ago
Comment on attachment 8582932 [details] [review]
[gaia] steveck-chung:dialer-withheld-number > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):bug 1131490
[User impact] if declined:Localization broken if call log contains withheld number.
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: N/A
Flags: needinfo?(schung)
Attachment #8582932 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8582932 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
This issue is verified fixed on the latest Nightly Flame KK  3.0 and 2.2 builds.

Actual Results: A call from a withheld number is now properly labeled in the call log.

Environmental Variables:
Device: Flame 3.0 KK (319MB) (Full Flash)
BuildID: 20150401010204
Gaia: 03164bd160809747e6a198e0dba1b7c3ee7789f5
Gecko: 18a8ea7c2c62
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

Environmental Variables:
Device: Flame 2.2 KK (319MB) (Full Flash)
BuildID: 20150401002624
Gaia: 8b3086ad3963f1707e2bee9094baccafffe161c4
Gecko: 20b67213a047
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.