980456 - [B2G][FTE][Date & Time] Continent and city selections aren't displayed highlighted

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Tony Nguyen]

Attachment: screenshot

Description:
When user changes continent and/or city on Date & Time screen during FTE/FTU, the locations don't appear highlighted in the image. 

Repro Steps:
1) Updated Buri to BuildID: 20140306040204
2) Open Settings App
3) Navigate to Device Information>More Information>Reset Phone
4) On FTE, select next until Date & Time screen is reached
5) Tap "Change" button next to current city
6) Select new city
7) Tap "OK"

Actual Result:
Timezone and city image does not reflect the changes made

Expected Result:
Timezone and city image reflects the changes made

Environmental Variables:
Device: Buri Master M-C mozRIL
BuildID: 20140306040204
Gaia: 9cb35e701df44766d9b3560b0defe0a401a0ecdd
Gecko: 8122ffa9e1aa
Version: 30.0a1
v1.2-device.cfg

Attached: screenshot

Comment 2

[Tony Nguyen]

Attachment: working_screenshot

I'm attaching a new screenshot to show the Date & Time working correctly on latest Buri build v1.3 side by side with the same page as Master M-C to better represent this issue. The continents are not being colored/highlighted in at all, this is not a dupe of bug 965912 and therefore I'm reopening. 

~Issue does not reproduce v1.3~
Device: Buri v1.3 mozRIL
BuildID: 20140307004004
Gaia: e8bf7326ce5ed63bc8ef8b2cff1eba094887b9bf
Gecko: 4efa30fe00e7
Version: 28.0
v1.2-device.cfg

Comment 3

[Matthew Vaughan]

The following regression window was found using b2g-inbound builds:

- Last Working -
Device: Buri ENG v1.4 MOZ RIL
BuildID: 20140305060225
Gaia: 2f72fbc4dd068bb6cce8d63b02c852fa19cb38d2
Gecko: c148486b6108
Version: 30.0a1
Firmware Version: V1.2-device.cfg

- First Broken -
Device: Buri ENG v1.4 MOZ RIL
BuildID: 20140305061723
Gaia: 48e101917a73e228b27537d30254d276a4721dae
Gecko: 2fc31a6b0cbf
Version: 30.0a1
Firmware Version: V1.2-device.cfg

**This is a gaia issue**
Push log: https://github.com/mozilla-b2g/gaia/compare/2f72fbc4dd068bb6cce8d63b02c852fa19cb38d2...48e101917a73e228b27537d30254d276a4721dae

Comment 4

[Jason Smith [:jsmith]]

Broken by bug 975637.

Comment 5

[Francesco Lodolo [:flod]]

I have a patch working locally, but I can't add attachments to bugzilla or request review
https://github.com/mozilla-b2g/gaia/pull/17038

Tested and working locally on Keon.

Old hardcoded 'UTC' was used to generate the CSS class name that highlights the region.

Comment 6

[Francesco Lodolo [:flod]]

Travis is dying on connection problems, opening a needinfo with the original reviewer
https://github.com/mozilla-b2g/gaia/pull/17038

Comment 7

[Francesco Lodolo [:flod]]

Attachment: Screenshot with patch applied

Comment 8

[Francesco Lodolo [:flod]]

Attachment: Pull request

Old code used regexp to change strings like "UTC+05:00" to "UTC0500", to obtain the class name used to highlight regions.

New code applied the regexp only to "+05:00", resulting in the wrong class name.

Note: also need someone to check-in this patch if bug 975637 isn't backed out in the meantime, since this is considered a blocking regression (see bug 975637 comment 40).

Comment 9

[Gregor Wagner [:gwagner]]

Comment on attachment 8388677 [details] [review]
Pull request

I think this was removed by mistake?

Comment 10

[Francesco Lodolo [:flod]]

(In reply to Gregor Wagner [:gwagner] from comment #9)
> Comment on attachment 8388677 [details] [review]
> Pull request
> 
> I think this was removed by mistake?

Did I remove the review flag? I though I was removing only the needinfo.
I'll ping fcampo or a FTE peer this morning on IRC to speed things up.

Comment 11

[Fernando Campo (:fcampo)]

Comment on attachment 8388677 [details] [review]
Pull request

Code is OK (sorry I missed that in the first review), but it needs a test. 
Should be simple, something in the line of:

var timezoneOverlay = document.getElementById('time_zone_overlay');

test('should highlight timezone', function() {
  timezoneOverlay.className = 'UTC' + FAKE_TIMEZONE.utcOffset.replace(/[+:]/g, '');
});

Anyway, r+ proven the test is added and travis green after that.
Thanks for the quick fix!

Comment 12

[Francesco Lodolo [:flod]]

Thanks, added test, now waiting for Travis.

Comment 13

[Francesco Lodolo [:flod]]

https://travis-ci.org/mozilla-b2g/gaia/builds/20520445

Setting check-in needed

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Master: a6fed6722d18920cbe880b811e2a9dbeea46bfe7

Comment 15

[Francesco Lodolo [:flod]]

Verified on Keon, Gaia Git commit 905c9409 (2014-03-11 14:26:17): regions are correctly highlighted when region/city change.

Comment 16

[Francesco Lodolo [:flod]]

(In reply to Tony Nguyen from comment #0)
> 3) Navigate to Device Information>More Information>Reset Phone

Forgot one note for the reporter: you don't need to reset the phone, just go to Settings-Developer, there's a button to start FTE there.