Closed Bug 1139735 Opened 10 years ago Closed 10 years ago

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

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

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

VERIFIED FIXED
2.2 S9 (3apr)
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: hcheng, Assigned: marta)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files, 1 obsolete file)

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)
(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!
blocking-b2g: 2.2? → ---
taking
Assignee: nobody → marta
Flags: needinfo?(marta)
Attached file [gaia] martasect:Bug_1139735 > mozilla-b2g:master (obsolete) (deleted) —
Comment on attachment 8578576 [details] [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] [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+
Comment on attachment 8578576 [details] [gaia] martasect:Bug_1139735 > mozilla-b2g:master corrected. Ben, can you r+ it?
Attachment #8578576 - Flags: review?(bfrancis)
Comment on attachment 8578576 [details] [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)
Flags: needinfo?(marta)
Keywords: checkin-needed
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.
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
Closed: 10 years ago
Resolution: --- → FIXED
Sorry about that, strange as the gaia-try has a passing Gu test. I was too optimistic here.
Flags: needinfo?(kgrandon)
.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.
Hi Marta, Could you check whether comment 17 from Sam helps? Thanks!
Status: REOPENED → NEW
Attachment #8578576 - Attachment is obsolete: true
Flags: needinfo?(marta)
Comment on attachment 8580599 [details] [gaia] martasect:Bug_1139735 > mozilla-b2g:master Sam, could you?
Attachment #8580599 - Flags: feedback?(sfoster)
Comment on attachment 8580599 [details] [gaia] martasect:Bug_1139735 > mozilla-b2g:master See comments in the PR.
Attachment #8580599 - Flags: feedback?(sfoster)
Comment on attachment 8580599 [details] [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] [gaia] martasect:Bug_1139735 > mozilla-b2g:master Looks good to me. Thanks.
Attachment #8580599 - Flags: review?(kgrandon) → review+
Status: NEW → RESOLVED
Closed: 10 years ago10 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
Flags: needinfo?(hcheng)
Whiteboard: [systemsfe]javascript:void(0); → [systemsfe]
Comment on attachment 8580599 [details] [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)
Attachment #8580599 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
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
Comment on attachment 8642991 [details] [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] [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] [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.

Attachment

General

Created:
Updated:
Size: