Closed
Bug 834515
Opened 12 years ago
Closed 12 years ago
Updating to a packaged app with a new name via introducing a locale override allows changing the app name
Categories
(Core Graveyard :: DOM: Apps, defect)
Tracking
(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)
People
(Reporter: jsmith, Assigned: ochameau)
References
Details
Attachments
(1 file, 1 obsolete file)
Build: B2G 18 1/24/2013
Device: Unagi
Steps:
1. Install a packaged app with app name X and no locale override for the app name
2. Update the app on the server to the same app name X with a locale override with app name Y that matches the phone's locale
3. Check for updates
4. Apply the update
Expected:
The app name should be still be the old name from step #1 - app name X.
Actual:
The app name is the locale overridden app name - app name Y.
Reporter | ||
Comment 1•12 years ago
|
||
At this point in the game we probably can't block on this. But we should probably try to get this in for v1.01.
tracking-b2g18:
--- → ?
Reporter | ||
Updated•12 years ago
|
Blocks: b2g-app-updates
Ugh, this basically means that the changes in bug 826555 aren't working at all :( It's trivial to work around the checks there by playing around with locales. I think this is a blocker.
blocking-b2g: --- → tef?
Assignee | ||
Comment 3•12 years ago
|
||
Opps, looks like I made a mistake while addressing review comment from bug 826555.
http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/AppsUtils.jsm#262
let defaultName = new ManifestHelper(aManifest, aApp.origin).name;
should be
let defaultName = new ManifestHelper(previousManifest, aApp.origin).name;
But that doesn't explain this bug and bug 834518. It allows to rename the app with a new locale matching the os phone's locale, but the selected name will be the default new locale specified in manifest:
From: { name: 'X' }.
To: { name: 'Z', { locale: { en: { name: 'Y' } } }.
The name will switch from X to Z.
I'll try to figure out what wrong while addressing that obvious error.
Updated•12 years ago
|
Assignee: nobody → poirot.alex
blocking-b2g: tef? → tef+
Comment 4•12 years ago
|
||
Marking status-b2g18 and status-b2g18-v1.0.0 as affected, please update the status to fixed once this is verified landed on v1-train/mozilla-b2g18 and v1.0.0/mozilla-b2g18_v_1_0_0
Assignee | ||
Comment 5•12 years ago
|
||
My unagi just died, so I wasn't able to finalize and test properly this patch.
But here is why I've tried to fix:
- In AppsUtils.normalizeManifest, aApp.manifest ends up being null. So that we should somehow retrieve the current app manifest (i.e. before the update). For that I modified checkForUpdate and factorize the call to _readManifest in order to be able to pass it to normalizeManifest.
- Instead of calling AppsUtils.normalizeManifest throught AppUtils.checkManifest in all scenarios: install, download and updates, I only call it from checkForUpdates. I ended up renaming this function to AppUtils.ensureSameAppName.
I'll try to borrow someone else phone while being at FOSDEM, but if anyone has free cycles, do not hesitate to take over this bug and the related one.
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #708833 -
Attachment is obsolete: true
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 709719 [details] [diff] [review]
Bug 834515 - Updating to a packaged app with a new name via introducing a locale override allows changing the app name r=fabrice
Desktop tests seems to pass but they crash at the end of test with a false assertion:
WARNING: NS_ENSURE_TRUE(!(exists)) failed: file /mnt/desktop/gecko/toolkit/profile/nsToolkitProfileService.cpp, line 650
^G###!!! ASSERTION: bad size recorded: 'aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize', file /mnt/desktop/gecko/xpcom/base/nsTraceRefcntImpl.cpp, line 441
NS_LogAddRef_P (/mnt/desktop/gecko/xpcom/base/nsTraceRefcntImpl.cpp:970)
nsAlertsService::AddRef() (/mnt/desktop/gecko/toolkit/system/gnome/nsAlertsService.cpp:10)
nsAlertsServiceConstructor (/mnt/desktop/gecko/toolkit/system/gnome/nsGnomeModule.cpp:26)
nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) (/mnt/desktop/gecko/xpcom/components/nsComponentManager.cpp:1036)
nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) (/mnt/desktop/gecko/xpcom/components/nsComponentManager.cpp:1427)
nsGetServiceByContractID::operator()(nsID const&, void**) const (/mnt/desktop/gecko/obj-x86_64-unknown-linux-gnu/xpcom/build/nsComponentManagerUtils.cpp:247)
nsCOMPtr<nsIAlertsService>::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) (/mnt/desktop/gecko/obj-x86_64-unknown-linux-gnu/toolkit/components/alerts/../../../dist/include/nsCOMPtr.h:1184)
I'm really not expecting to introduce such issue with this patch.
Fabrice, see comment 5 for more details on what I done in this patch.
Attachment #709719 -
Flags: review?(fabrice)
Comment 8•12 years ago
|
||
Comment on attachment 709719 [details] [diff] [review]
Bug 834515 - Updating to a packaged app with a new name via introducing a locale override allows changing the app name r=fabrice
Review of attachment 709719 [details] [diff] [review]:
-----------------------------------------------------------------
That looks good to me, but I need some time to run tests.
Comment 9•12 years ago
|
||
We're continuing to block on this due to the security impact.
Comment 11•12 years ago
|
||
Comment on attachment 709719 [details] [diff] [review]
Bug 834515 - Updating to a packaged app with a new name via introducing a locale override allows changing the app name r=fabrice
Review of attachment 709719 [details] [diff] [review]:
-----------------------------------------------------------------
That looks good to me, but I need some time to run tests.
Attachment #709719 -
Flags: review?(fabrice) → review+
Comment 12•12 years ago
|
||
Did the tests pass? Does it look like this'll be landing soon?
Comment 13•12 years ago
|
||
Batch edit: Bugs marked status-b2g18: affected after 2/13 branching of v1.0.1 are now also status-b2g18-v1.0.1: affected
status-b2g18-v1.0.1:
--- → affected
Comment 14•12 years ago
|
||
Comment 15•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment 16•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/2fce2dcc4e2e
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/459612cc72d5
Reporter | ||
Comment 17•12 years ago
|
||
Tests:
- Rename "name" parameter on update (used packaged app) - PASS, but hit bug 834672 on the way
- Rename "name" parameter on locale override name on update - FAIL, bug 844369
- Add a locale override to override name to something different on update - PASS
Two followups:
- bug 844369
- bug 844243
Status: RESOLVED → VERIFIED
Keywords: verifyme
Updated•7 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•