[Settings] "short_name" is not used in "Settings>Privacy Controls>Transparency Control>Applications"

VERIFIED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::Settings
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Hermes Cheng (inactive after July 27, 2015), Assigned: marta)

Tracking

(Blocks: 2 bugs)

unspecified
2.2 S9 (3apr)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments, 1 obsolete attachment)

As bug 1134680, short_name should be used in Settings app. However, when I go to "Settings>Privacy Controls>Transparency Control>Applications", it shows "name" but not "short_name".
Blocks: 1134680
blocking-b2g: --- → 2.2?
Whiteboard: [systemsfe]
Blocks: 1001861
Wilfred, can you confirm that this is needed? Team thought privacy controls was removed in 2.2 anyway.
Flags: needinfo?(wmathanaraj)
Marta, can you take this one? Ben Francis can help out if you have any questions.
Flags: needinfo?(marta)

Comment 3

3 years ago
(In reply to Candice Serran (:cserran) from comment #1)
> Wilfred, can you confirm that this is needed? Team thought privacy controls
> was removed in 2.2 anyway.

Hi Candice,
privacy controls is not blocking 2.2 and will be fix by DT.
Thanks!
Blocks: 1057675
blocking-b2g: 2.2? → ---
(Assignee)

Comment 4

3 years ago
taking
Assignee: nobody → marta
Flags: needinfo?(marta)
Created attachment 8578576 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master
(Assignee)

Comment 6

3 years ago
Comment on attachment 8578576 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

Here is a patch. I have included the manifest.short_name the way it was done in bug 1134680, as an alternative to the manifest.name
Ben, could you have a look and let me know if that was the right way?
Attachment #8578576 - Flags: feedback?(bfrancis)
Comment on attachment 8578576 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

Thanks for the patch!

This is fine, but you might consider using ManifestHelper.displayName as implemented in 1137501 instead, this allows us to keep this logic in one place. We may update all the other places to use this too.
Attachment #8578576 - Flags: feedback?(bfrancis) → feedback+
(Assignee)

Comment 8

3 years ago
Comment on attachment 8578576 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

corrected. Ben, can you r+ it?
Attachment #8578576 - Flags: review?(bfrancis)
Comment on attachment 8578576 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

That's great, thanks.

I've just restarted some tests on Treeherder that look like infrastructure problems, then we can add checkin-needed.
Attachment #8578576 - Flags: review?(bfrancis) → review+
TreeHerder looks unhappy, you may need to rebase this pull request against master.
Flags: needinfo?(marta)
(Assignee)

Updated

3 years ago
Flags: needinfo?(marta)
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
(Assignee)

Comment 12

3 years ago
I did it, still won't land...?
Similar problem as we don't really have a privacy panel component yet. 

Let's try landing manually for now: https://github.com/mozilla-b2g/gaia/commit/6e24aff11fbb385a6fb1651536a97263457b6c50
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-master: --- → fixed
Resolution: --- → FIXED
I had to revert this in https://github.com/mozilla-b2g/gaia/commit/7dc475d86c5b26c2348cc012509a47618e75a205 for Gu test failures:
https://treeherder.mozilla.org/logviewer.html#?job_id=1549749&repo=b2g-inbound
Status: RESOLVED → REOPENED
Flags: needinfo?(marta)
Resolution: FIXED → ---

Updated

3 years ago
Flags: needinfo?(kgrandon)
Sorry about that, strange as the gaia-try has a passing Gu test. I was too optimistic here.
Flags: needinfo?(kgrandon)
Created attachment 8580599 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master
.displayName is a property of the ManifestHelper, and _makeAppRepresentation is using the manifest directly. This shouldn't have passed unit tests - might need to look at those to figure out why it wasnt caught. If you grep for `new ManifestHelper` you'll find several examples of how its used; refactoring to use that may be the right thing to do here.

Updated

3 years ago
status-b2g-v2.2: --- → affected

Comment 18

3 years ago
Hi Marta,
Could you check whether comment 17 from Sam helps?
Thanks!
Status: REOPENED → NEW
(Assignee)

Updated

3 years ago
Attachment #8578576 - Attachment is obsolete: true
Flags: needinfo?(marta)
(Assignee)

Comment 19

3 years ago
Comment on attachment 8580599 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

Sam, could you?
Attachment #8580599 - Flags: feedback?(sfoster)
Comment on attachment 8580599 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

See comments in the PR.
Attachment #8580599 - Flags: feedback?(sfoster)
(Assignee)

Comment 21

3 years ago
Comment on attachment 8580599 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

Kevin, I know Sam is on PTO, could you please review? I addressed all Sam's remarks and fixed the tests. Now should work.
Attachment #8580599 - Flags: review?(kgrandon)
Comment on attachment 8580599 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

Looks good to me. Thanks.
Attachment #8580599 - Flags: review?(kgrandon) → review+
In master: https://github.com/mozilla-b2g/gaia/commit/70ce8d87be7df4dbeff34c3ebde787a55905de6c
Status: NEW → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
NI myself for following verification.
Flags: needinfo?(hcheng)
Whiteboard: [systemsfe] → [systemsfe]javascript:void(0);
Verified master branch
* master
Build ID               20150329160203
Gaia Revision          67ad91f3f660b1f16b354ee4c5159ddc5a74d149
Gaia Date              2015-03-28 10:02:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/385840329d91
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150329.191719
Firmware Date          Sun Mar 29 19:17:28 EDT 2015
Bootloader             L1TC100118D0
status-b2g-master: fixed → verified
Flags: needinfo?(hcheng)
Whiteboard: [systemsfe]javascript:void(0); → [systemsfe]
(Assignee)

Comment 26

3 years ago
Comment on attachment 8580599 [details] [review]
[gaia] martasect:Bug_1139735 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):1139735
[User impact] if declined:the issue is fixed on Master so we need backward compatibility
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: non
Attachment #8580599 - Flags: approval-gaia-v2.2?(dietrich)
Flags: needinfo?(wmathanaraj)
Attachment #8580599 - Flags: approval-gaia-v2.2?(dietrich) → approval-gaia-v2.2?(bbajaj)

Updated

3 years ago
Attachment #8580599 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/d4d2c897968257565a005642a56ffa884de9c983
status-b2g-v2.2: affected → fixed
Target Milestone: --- → 2.2 S9 (3apr)
verified, and add test case to v2.2 on MozTrap.

Build ID               20150412162502
Gaia Revision          cec00d643f517ffd96cde559cd3bbd43ab85816c
Gaia Date              2015-04-10 21:41:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5005522fd68e
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150412.200029
Firmware Date          Sun Apr 12 20:00:40 EDT 2015
Bootloader             L1TC000118D0
Status: RESOLVED → VERIFIED
status-b2g-v2.2: fixed → verified
Created attachment 8642991 [details] [review]
[gaia] albertopq:fix-privacy-test > mozilla-b2g:master
Comment on attachment 8642991 [details] [review]
[gaia] albertopq:fix-privacy-test > mozilla-b2g:master

Kevin, as you reviewed the original patch, could you please take a look to this? It seems that the unit test are failing in master.
Attachment #8642991 - Flags: review?(kevingrandon)
Comment on attachment 8642991 [details] [review]
[gaia] albertopq:fix-privacy-test > mozilla-b2g:master

Oddly though, this test started passing in master once bug 1186301 was reverted - is it still needed?

Should we reland this as well as bug 1186301? Please flag again for review if needed, looks fine to me as long as tests pass.
Flags: needinfo?(dwi2)
Attachment #8642991 - Flags: review?(kevingrandon)
I cannot see the failing test as PASS in the GU13 logs after the backout. May be is running in other chunk? I'm not sure how the tests are splited in the GU.

Anyway, I it seems that with both, the test runs and passes:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=88ca4cc0b13ec33313f051167395471cbdb6fd38
My observation is the same with :albertopq.
The failing test '/privacy-panel/test/unit/app_list_test.js' never runs in treeherder. It seems that app_list_test.js usually get skipped. I randomly pick two jobs for example:

1. 
https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/_qxsmxRnS2-cdRwglK7Y6w/0/public/logs/live_backing.log

  ...
Running tests:  [ '/music/test/unit/metadata/mp4_test.js',
  ...
  '/privacy-panel/test/unit/ala/define_custom_location_test.js',
  '/privacy-panel/test/unit/app_list_test.js',
  '/privacy-panel/test/unit/rp/auth_test.js' ]
...
TEST-START | null | [privacy-panel-test/unit/ala/define_custom_location_test.js] ALA CustomLocation should validate false when longitude is empty
TEST-PASS | null | [privacy-panel-test/unit/ala/define_custom_location_test.js] ALA CustomLocation should validate false when longitude is empty
TEST-END | null | [privacy-panel-test/unit/ala/define_custom_location_test.js] ALA CustomLocation should validate false when longitude is empty took 0 ms
TEST-START | null | [privacy-panel-test/unit/rp/auth_test.js] RP Auth panels validate passphrase should validate given passphrase during register
TEST-PASS | null | [privacy-panel-test/unit/rp/auth_test.js] RP Auth panels validate passphrase should validate given passphrase during register
TEST-END | null | [privacy-panel-test/unit/rp/auth_test.js] RP Auth panels validate passphrase should validate given passphrase during register took 0 ms
...


2. 
https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/cac-zad_Tv2p9jPD3muwqg/0/public/logs/live_backing.log

...
Running tests:  [ '/music/test/unit/tabbar_test.js',
  ...
  '/privacy-panel/test/unit/ala/define_custom_location_test.js',
  '/privacy-panel/test/unit/app_list_test.js' ]
...
TEST-START | null | [privacy-panel-test/unit/ala/define_custom_location_test.js] ALA CustomLocation should validate false when longitude is empty
TEST-PASS | null | [privacy-panel-test/unit/ala/define_custom_location_test.js] ALA CustomLocation should validate false when longitude is empty
TEST-END | null | [privacy-panel-test/unit/ala/define_custom_location_test.js] ALA CustomLocation should validate false when longitude is empty took 0 ms
*~*~* Results *~*~*
passed: 171
failed: 0
todo: 0

  171 passing (2ms)
I also try to run app_list_test on my laptop, without patch from :albertopq. However test agent tells me no test was run:
Running tests:  [ '/privacy-panel/test/unit/app_list_test.js' ]

  0 passing (0ms)

But I got this log from nightly:

JavaScript error: app://privacy-panel.gaiamobile.org/js/vendor/alameda.js?time=1438757617501, line 537: Error: Not loaded: app://system.gaiamobile.org/shared/test/unit/mocks/mock_manifest_helper.js


My guess is that app_list_test already got skipped for some unknown reason for a long time, but my patch for bug 1186301 accidentally changes test chunk and makes app_list_test to be the first test in the chunk

Running tests:  [ '/privacy-panel/test/unit/app_list_test.js',
  '/privacy-panel/test/unit/rp/auth_test.js',
  ...

(see https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/UoL66wjDTsqXPmyiJDj9hA/2/public/logs/live_backing.log)

So I think the patch from :albertopq is necessary since this test is already broken but just accidentally got skipped for a long time.
Flags: needinfo?(dwi2)
Hi Kevin,

According to comment #33 and comment #34, could you help to review the patch? Thanks

Bugzilla did not allow me to flag review for this patch https://bugzilla.mozilla.org/attachment.cgi?id=8642991, that's why I flag NI.
Flags: needinfo?(kevingrandon)
Comment on attachment 8642991 [details] [review]
[gaia] albertopq:fix-privacy-test > mozilla-b2g:master

Alright, let's get these landed then. Thanks!
Flags: needinfo?(kevingrandon)
Attachment #8642991 - Flags: review+
You need to log in before you can comment on or make changes to this bug.