1075200 - [Settings] Unable to set AM/PM time when switching from 24 to 12 hour time format

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Seth Miko [:SMiko]]

Attachment: AMPM.txt

Description:
After switching the time format from 24 hr. to 12 hr.in a non English language, the user is unable to set AM/PM

Repro Steps:
1: Update a Flame to 20140930000203
2: Open Settings and change the language to French.
3: From Settings, open Time & Date
4: Toggle "Set automatically" and tap on current time.
5: Back out to "Time & Date" and change the time format to 12 hr.
4: Set the time and attempt to change the AM/PM option

Actual:
The user cannot change AM/PM. The dial never lands on the selection.

Expected:
The user is able to set AM/PM

Flame 2.1(319mb/full flash)

Device: Flame 2.1
BuildID: 20140930000203
Gaia: a00d102abfe8ae15c4fd14771efa2335c6d3b8d9
Gecko: cde28bd9a285
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Notes:
This issue also occurs in Bosnian.
This issue only occurs after viewing the time in 24 hr format before switching. (Step 4)

Repro frequency: 100%

See attached: logcat

Video clip: http://youtu.be/b39gcjCwGMw

Comment 1

[Seth Miko [:SMiko]]

This issue DOES repro on Flame 2.2 (319mb/full flash) Flame 2.1(512mb/full flash) 
Actual result: The user cannot change AM/PM. The dial never lands on the selection.

Flame 2.2 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140930040206
Gaia: 77ef35f5429bc3dfe9ca192b9aacc3c0bf8857de
Gecko: 7c24470b6b3a
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1 KitKat Base (512mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20140930000203
Gaia: a00d102abfe8ae15c4fd14771efa2335c6d3b8d9
Gecko: cde28bd9a285
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
_____________________________________________________________________________________________
This issue does NOT occur on Flame 2.0 as the 12/24hr clock format option does not exist.

Flame 2.0 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.0
BuildID: 20140930000204
Gaia: 5c2303ec4e367da060aa1b807d541a6549b3d72a
Gecko: 38a496c50d98
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

The user should be able to set the time manually, regardless of which language they are in. NI Settings owner to see if this should block.

Comment 3

[Gerry Chang [:gchang]]

[Blocking Requested - why for this release]:
Hi Howie,
I can also reproduce this. Need your input for this to see if this is 2.1 blocker.

Comment 4

[howie [:howie]]

Thank you Gerry. This is a blocker as major function defect.

Comment 5

[howie [:howie]]

Hi Fred, can you help on this? Thanks.

Comment 6

[[e-mail]]

Seems html item selection is changed, but UI does not visual reflect the change.

Comment 7

[[e-mail]]

When error is happening after scroll, the valuePicker element (aria-valuenow, aria-valuetext) shows wrong data, and translateY goes to some wild value

```
<div aria-valuetext="undefined" aria-valuenow="NaN" aria-valuemax="1" aria-valuemin="0" role="spinbutton" class="value-picker-hour24-wrapper">
  <div style="transform: translateY(-107.5px);" class="value-picker-hour24-state animation-on"><div class="picker-unit">AM</div><div class="picker-unit">PM</div></div>
</div>
```

let's check what's happened in value_picker vp_touchmove / vp_touchend during scroll

Comment 8

[[e-mail]]

The bug is also reproducible in any lanugage via following steps:

In DateTime panel,

1. change timeformat from 12h->24h
2. open time selector (must)
3. change timeformat from 24h->12h.
4. open time selector, the AM/PM scroll work incorrectly

Comment 9

[[e-mail]]

value_picker 's `tunedIndex` is missing (NaN or Infinite) in fail cases. digging...

Comment 10

[[e-mail]]

The root cause is the AM/PM element's clientHeight return 0 when dom is not ready, it cause the devide zero (N/0) error.

Comment 11

[Ken Chang[:kenkai]]

ni?me to track this issue.

Comment 12

[[e-mail]]

Attachment: pull request redirect to github

* workaround of clientHeight return 0 issue
* fix potential bugs

patch to meet the 2.0 timeframe.


Still checking why dom is not ready in this case

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8501582 [details] [review]
pull request redirect to github

please try to fix the real problem if possible

Comment 14

[[e-mail]]

Comment on attachment 8501582 [details] [review]
pull request redirect to github

Add check and get unit height when needed so we don't have to hardcode the height.

If dom is not ready in the first call, the issue will be auto corrected in next call.

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8501582 [details] [review]
pull request redirect to github

We need unit test before landing this since it's a little complex. What's the problem that this comes without tests?

Comment 16

[[e-mail]]

Unfortunately current valuePicker implementation is not stub-able, 
we have to refactor valuePicker to make it stub-able, so we could test issues in this granularity.

Comment 17

[[e-mail]]

I added more test for valuePicker and add comment in source code to note follower that this edge case should be noticed.

Comment 18

[Seth Miko [:SMiko]]

This issue also occurs in English on Flame 2.1 (319mb/full flash) and Flame 2.2 (319mb/full flash)

Actual result: The user cannot change AM/PM. The dial never lands on the selection.

Device: Flame 2.2
BuildID: 20141012040203
Gaia: 717ad4e8b7fc10ab8248500d00ba5ba0977fa8ab
Gecko: 44168a7af20d
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Device: Flame 2.1
BuildID: 20141012001201
Gaia: d18e130216cd3960cd327179364d9f71e42debda
Gecko: 610ee0e6a776
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 19

[[e-mail]]

tree herder green https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=1442256e2903

merged to master 
https://github.com/mozilla-b2g/gaia/commit/2a536e4df82410178d8440cc710d8f838a95a0b9

Comment 20

[[e-mail]]

Comment on attachment 8501582 [details] [review]
pull request redirect to github

[Approval Request Comment] 2.1 blocker
[String changes made]: None

Comment 21

[[:fabrice] Fabrice Desré]

(In reply to Fred Lin [:gasolin] from comment #20)
> Comment on attachment 8501582 [details] [review]
> pull request redirect to github
> 
> [Approval Request Comment] 2.1 blocker
> [String changes made]: None

Please answer the full questionnaire.

Comment 22

[[e-mail]]

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment] 2.1 blocker
[Bug caused by] (feature/regressing bug #): bug 1023735
[User impact] if declined: affect user done with reproducible steps
[Testing completed]: tree herder passed
[Risk to taking this patch] (and alternatives if risky): none
[String changes made]: none

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/a9e3549159fb52bff654724c0183d7342d527fbc

Comment 24

[Seth Miko [:SMiko]]

Verified fixed on Flame 2.2 (319mb/full flash) and Flame 2.1 (319mb/full flash)

Actual result: The user is able to set AM/PM

Device: Flame 2.2
BuildID: 20141017040208
Gaia: abef62c0623e5504a97b4fd411e879a67b285b52
Gecko: ae1dfa192faf
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 36.0a1 (2.2)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Device: Flame 2.1
BuildID: 20141017001201
Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98
Gecko: 2befa902ff5c
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0