1142572 - Use displayName from ManifestHelper to abstract short_name vs. name properties

Status: RESOLVED FIXED

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

Description

[Sam Foster [:sfoster] (he/him)]

Since the patch in bug 1137501 landed, we have a displayName property to better abstract away the use of short_name vs. name from the manifest. We should go back and fix those `name || short_name` uses for consistency and simplicity's sake.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

If I'm understanding this bug correctly, I think this is the reason why l10n is broken for translations for apps?

Comment 2

[Sam Foster [:sfoster] (he/him)]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #1)
> If I'm understanding this bug correctly, I think this is the reason why l10n
> is broken for translations for apps?

If the ManifestHelper or local_name changes are implicated in broken translations (which I dont *think* it is?) we should look at some of the other dependants of bug 1001861 where these changes were originally implemented. Making the changes called for by this bug shouldnt fix anything broken in those other bug; this is just some code clean up.

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] sfoster:manifest-displayname-bug-1142572 > mozilla-b2g:master

Comment 4

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8580895 [details] [review]
[gaia] sfoster:manifest-displayname-bug-1142572 > mozilla-b2g:master

This bug covers going back and consolidating our use of the ManifestHelper's displayName property instead of (short_name | name) logic. 

r? alive for system AppWindow and (Card view) Card parts. As we've pushed the short_name vs name down to the ManifestHelper, we can get rid of the AppWindow.prototype.shortName entirely. It had a short life. 

r? christian for bookmark editor
r? arthur for settings

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8580895 [details] [review]
[gaia] sfoster:manifest-displayname-bug-1142572 > mozilla-b2g:master

Looks good to me, thanks.

Comment 6

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8580895 [details] [review]
[gaia] sfoster:manifest-displayname-bug-1142572 > mozilla-b2g:master

Good job, thanks a lot

Comment 7

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

Comment on attachment 8580895 [details] [review]
[gaia] sfoster:manifest-displayname-bug-1142572 > mozilla-b2g:master

Thanks!

Comment 8

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#pFs2EBqvSQKlJd8Wzlqrcw

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

Comment 9

[Sam Foster [:sfoster] (he/him)]

Gaia currently closed, r+'d, tests are green, please land when it opens

Comment 10

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/29023

Autolander could not land the pull request due to not having collaborator rights. This is possibly due to a tree closure. Please check the tree status and request checkin again once the tree is open.

Comment 11

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#mi6GtyXST3eJb0S2gyNw-w

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

Comment 12

[Sam Foster [:sfoster] (he/him)]

This patch has had a green test run on https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=dedd57b4d1917c10f6017725454077bb549daa03, but trying to land since gaia re-opened and I get a bunch of taskcluster failures, like: 

fatal: unable to access 'https://github.com/mozilla-b2g/gaia/': transfer closed with outstanding read data remaining    

I'll close and re-open the PR to give it another shot, but as I'm out on PTO could you push this over the finish line? Thanks!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/6b95562a08cd4603ec269c92938e392f0b4d1ea3

Comment 14

[Kevin Grandon :kgrandon]

Backed out for Gij1 failures on TC.

https://github.com/mozilla-b2g/gaia/commit/37cfde85cdf74142fdf1f1dec5f1329e8f8d6352

Revert commit: https://github.com/mozilla-b2g/gaia/commit/37cfde85cdf74142fdf1f1dec5f1329e8f8d6352

Adding a needinfo to myself to try to shepherd this to landing after a green run.

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:reland_bug_1142572 > mozilla-b2g:master

Comment 16

[Kevin Grandon :kgrandon]

An example failure: https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/exTk3ywNQ5WOLPX55COY6A/1/public/logs/live_backing.log

Comment 17

[Kevin Grandon :kgrandon]

Attachment: Screenshot of timeout at bookmark save

Ben - it looks like Sam is out and I'm not entirely sure what the correct behavior is supposed to be here. This is a screenshot when the test times out. Does this screen look correct to you? Do you want to look at this before Sam gets back from PTO?

Comment 18

[Ben Francis [:benfrancis]]

I've commented on the pull request with what I think the problem is. I'm not sure when Sam is back but I can take this if he doesn't get back soon.

Comment 19

[GitHub Autolander Bot]

Attachment: [gaia] sfoster:manifest-displayname-r2-bug-1142572 > mozilla-b2g:master

Comment 20

[Sam Foster [:sfoster] (he/him)]

(In reply to Ben Francis [:benfrancis] from comment #18)
> I've commented on the pull request with what I think the problem is. I'm not
> sure when Sam is back but I can take this if he doesn't get back soon.

Thanks for catching that Ben (and Gij!), it was my oversight not noticing WebManifestHelper vs. ManifestHelper. I think that should be good to go now it the tests go green.

Comment 21

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8586337 [details] [review]
[gaia] sfoster:manifest-displayname-r2-bug-1142572 > mozilla-b2g:master

Sorry to bug you again, but attachment #8580895 [details] [review] landed then was backed out for causing Gij failures. I've removed the erroneous bookmarks app changes - the rest of the patch is unchanged from what you saw before.

Comment 22

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8580895 [details] [review]
[gaia] sfoster:manifest-displayname-bug-1142572 > mozilla-b2g:master

This patch landed but backed out. Updated PR in attachment #8586337 [details] [review]

Comment 23

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

Comment on attachment 8586337 [details] [review]
[gaia] sfoster:manifest-displayname-r2-bug-1142572 > mozilla-b2g:master

WFM

Comment 24

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/ece334a1a93af6c77b423ba9bdf5a3a0d0712eca