Closed Bug 637487 Opened 9 years ago Closed 9 years ago

Unable to install extensions when Fennec moved to SD Card

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: wesj, Assigned: wesj)

References

(Blocks 1 open bug)

Details

(Whiteboard: [needs-landing])

Attachments

(2 files, 1 obsolete file)

When I have Fennec moved to the SD card and try to install addons (restartless or not) I see errors in the console, most of which seem similar to:

uncaught exception: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.permissions]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: recursiveRemove :: line 1087"  data: no]
I think this was likely caused by bug 615519.
tracking-fennec: --- → ?
ERROR addons.xpi: Failed to remove file /mnt/sdcard/Android/data/org.mozilla.fennec/files/mozilla/kionm1bo.default/extensions/trash/phony@mbrubeck.limpet.net.xpi: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.permissions]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: recursiveRemove :: line 1089"  data: no]
resource://gre/modules/XPIProvider.jsm

This includes the patch where the problem occurs when I try to install phony.
tracking-fennec: ? → 2.0+
Assignee: nobody → wjohnston
Attached patch Patch v1Splinter Review
This is a bit of hacky way to do this, just for the AddonsManager. I've got a different patch in the works that to make this not throw on Android.
Comment on attachment 516014 [details] [diff] [review]
Patch v1

The more I think about this, callers who care should probably have the chance to catch these errors and do something about them, so I'm going to leave this as is and put it up for review.
Attachment #516014 - Flags: review?(dtownsend)
Comment on attachment 516014 [details] [diff] [review]
Patch v1

Ugh, why does this fail on Android on FAT32 but not on Windows on FAT32?
Attachment #516014 - Flags: review?(dtownsend) → review+
Windows uses nsLocalFileWin.cpp while, Android is traveling through nsLocalFileUnix.cpp. None of our file apis seem particularly smart about adjusting to different file systems. Just different OS's.

I'll push this to try just to be safe.
Wes, can you try this patch?
Attachment #516155 - Attachment is obsolete: true
Attachment #516157 - Flags: review?(doug.turner)
Comment on attachment 516157 [details] [diff] [review]
patch to bail when setting FAT prefs

I am not sure why you want to mask the valid error code inside local file.  Callers may want to know if changing a file permission fails.  I think you are saying, if it is FAT, we know that we can't change permissions per file in all case (which isn't totally true), so lets just return OK.  If that is the case, why is this ANDROID only?

Isn't it better to teach the callers to ignore the result of setPermission?
Whiteboard: [needs-review (dougt)]
Comment on attachment 516157 [details] [diff] [review]
patch to bail when setting FAT prefs

Please file a follow up bug to figure out what to do post FF4.  I think we want to do this on all unixy platforms.  Also, please add a commment under the #ifdef that points to this new bug number. 

You might want to use the STATFS macro.

Also you must test the return code of statfs.


r+  with those changes
Attachment #516157 - Flags: review?(doug.turner) → review+
(In reply to comment #7)
> Created attachment 516155 [details] [diff] [review]
> patch to bail when setting FAT prefs
> 
> Wes, can you try this patch?

Works great!
Whiteboard: [needs-review (dougt)]
Whiteboard: [needs-landing]
pushed http://hg.mozilla.org/mozilla-central/rev/378eec685e74
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Verified on:

Mozilla/5.0 (Android; Linux armv7l; rv:2.0b13pre) Gecko/20100307
Firefox/4.0b13pre Fennec/4.0b6pre
Status: RESOLVED → VERIFIED
Duplicate of this bug: 642073
You need to log in before you can comment on or make changes to this bug.