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

Status: RESOLVED FIXED

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

Description

[Steve Chung [:steveck]]

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

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] steveck-chung:message-l10n-xfail-list > mozilla-b2g:master

Comment 2

[Steve Chung [:steveck]]

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.

Comment 3

[Oleg Zasypkin [: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!

Comment 4

[Steve Chung [:steveck]]

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

Comment 5

[Oleg Zasypkin [: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!

Comment 6

[Steve Chung [:steveck]]

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.

Comment 7

[Oleg Zasypkin [: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 8

[Oleg Zasypkin [:azasypkin]]

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!

Comment 9

[Steve Chung [:steveck]]

Oops I forgot to clean up this as well, thanks!

Comment 10

[GitHub Autolander Bot]

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.

Comment 11

[Steve Chung [:steveck]]

Landed in master manually because it seems like false alarm:

https://github.com/mozilla-b2g/gaia/commit/62025ac40ae93bbeccdeb2d0187b27d69e31477e

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

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.

Comment 13

[Oleg Zasypkin [:azasypkin]]

(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