If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[Messages][l10n] Fix file path changes in l10n xfail.list

RESOLVED FIXED

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: steveck, Assigned: steveck)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
After some refactoring for NG the file path changed but we forgot to fix the list in l10n precommit-hook.

Comment 1

2 years ago
Created attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master
(Assignee)

Updated

2 years ago
Assignee: nobody → schung
(Assignee)

Comment 2

2 years ago
Comment on attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

Hi Oleg, it's a tiny patch simply for fixing the file path. I think we already reduce the l10n.get call previously and we might need to wait for l20n for fixing them properly.
Attachment #8609173 - Flags: review?(azasypkin)
Comment on attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

Thanks for the quick fix! Looks good, but I think we still should exclude activity_handler from l10n check (left question at GitHub).

Thanks!
Attachment #8609173 - Flags: review?(azasypkin) → review+
Status: NEW → ASSIGNED
(Assignee)

Comment 4

2 years ago
Comment on attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

Hi Oleg, I updated the patch a little to remove the l10n.get in activity handler because I think it might be easy to replace with async l10n.formatValue API, but I got some problem in test[1]. Please tell me if you prefer to simply add in filed list or polish activity handler, thanks!

[1] https://github.com/mozilla-b2g/gaia/pull/30189/files#r31038068
Attachment #8609173 - Flags: review+ → feedback?(azasypkin)
Comment on attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

You know I always support refactoring, and I like this one (left few nits and one question about "mozSetMessageHandlerPromise" at GitHub)!

Thanks!
Attachment #8609173 - Flags: feedback?(azasypkin) → feedback+
(Assignee)

Comment 6

2 years ago
Comment on attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

Hi Oleg, thanks for the suggestions and I ask kanru about mozSetMessageHandlerPromise: This API was planned for some background system message and once the given promise is completed, system will kill the app(not implement yet). So we need to make sure this api is called only when app is no need to be launched.
Attachment #8609173 - Flags: review?(azasypkin)
(In reply to Steve Chung [:steveck] from comment #6)
> Comment on attachment 8609173 [details] [review]
> [gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master
> 
> Hi Oleg, thanks for the suggestions and I ask kanru about
> mozSetMessageHandlerPromise: This API was planned for some background system
> message and once the given promise is completed, system will kill the
> app(not implement yet). So we need to make sure this api is called only when
> app is no need to be launched.

Cool, thanks for clarifying!
Comment on attachment 8609173 [details] [review]
[gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

Looks good, just one nit at github.

Thanks for the cleanup!
Attachment #8609173 - Flags: review?(azasypkin) → review+
(Assignee)

Comment 9

2 years ago
Oops I forgot to clean up this as well, thanks!
Keywords: checkin-needed

Updated

2 years ago
Keywords: checkin-needed

Comment 10

2 years ago
http://docs.taskcluster.net/tools/task-graph-inspector/#1XzLxcS1RBujgIC34m9kWA

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

Comment 11

2 years ago
Landed in master manually because it seems like false alarm:

https://github.com/mozilla-b2g/gaia/commit/62025ac40ae93bbeccdeb2d0187b27d69e31477e
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Hey guys, I found a small bug in this patch:

In this line: https://github.com/mozilla-b2g/gaia/blob/7f5325649e416ab1737f855473b038105072a7c8/apps/sms/views/shared/js/activity_handler.js#L329

the Promise.resolve should be `Promise.resolve({raw: sender});`, otherwise it's trying to localize the raw number or name from contact book.
Flags: needinfo?(schung)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #12)
> Hey guys, I found a small bug in this patch:
> 
> In this line:
> https://github.com/mozilla-b2g/gaia/blob/
> 7f5325649e416ab1737f855473b038105072a7c8/apps/sms/views/shared/js/
> activity_handler.js#L329
> 
> the Promise.resolve should be `Promise.resolve({raw: sender});`, otherwise
> it's trying to localize the raw number or name from contact book.

Hey Zibi,

Thanks for discovering!

The PR that fixes this is in review right now (bug 1208532) - [1] :)

[1] https://github.com/mozilla-b2g/gaia/pull/32138/files#diff-060047b3f409146d53262e4c88395224R158
Flags: needinfo?(schung)
You need to log in before you can comment on or make changes to this bug.