904518 - Show low storage space error for SD cards

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect, P1)

Description

[leo.bugzilla.gecko]

1. Title : device storage free space check error 
2. precondition : internal storage full. free space 0.
                  Default media location setting : internal
                  SD card storage insert.
3. STR
   Start camera app -> recording video -> can't save video file.
   Start camera app -> take picture -> Picture not saved popup.

Expected :
   if internal storage is full. camera app how memory full pop-up.

Actual:
   camera can take picture. 

Repro frequency: 5/5 100%

Test Version Info.
Device : leo
Gaia revision = "c60e3507d9df160e006d2e25967d26df2dc543cb"
Gecko revision = "b70519784430835a3e5a1d417eb3a818340c1c1c"

Comment 1

[Jason Smith [:jsmith]]

This is definitely a non-blocker - we already show to the user a system-wise toast when internal storage gets low and then persist a notification indicating storage is low until 10 MB of free internal space is hit. Having an additional check in camera to improve error handling is a good idea, but a nice to have if a system-wise indicator is already present.

Comment 2

[leo.bugzilla.gecko]

when external SD have over 10 MB free space and internal SD have 3MB free space.
nothing show low space toast. 
if remove external SD, you can see toast.

(In reply to Jason Smith [:jsmith] from comment #1)
> This is definitely a non-blocker - we already show to the user a system-wise
> toast when internal storage gets low and then persist a notification
> indicating storage is low until 10 MB of free internal space is hit. Having
> an additional check in camera to improve error handling is a good idea, but
> a nice to have if a system-wise indicator is already present.

Comment 3

[Jason Smith [:jsmith]]

(In reply to leo.bugzilla.gecko from comment #2)
> when external SD have over 10 MB free space and internal SD have 3MB free
> space.
> nothing show low space toast. 
> if remove external SD, you can see toast.
> 
> (In reply to Jason Smith [:jsmith] from comment #1)
> > This is definitely a non-blocker - we already show to the user a system-wise
> > toast when internal storage gets low and then persist a notification
> > indicating storage is low until 10 MB of free internal space is hit. Having
> > an additional check in camera to improve error handling is a good idea, but
> > a nice to have if a system-wise indicator is already present.

That's not what I'm seeing. I just tested this on a 8/13 Comm RIL build and tried to write past the 5 MB limit down 3 MB to see if we block the write. We did successfully block the write, which means our low storage detection is working. My testing I did was with a sdcard present as well.

Comment 4

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #3)
> (In reply to leo.bugzilla.gecko from comment #2)
> > when external SD have over 10 MB free space and internal SD have 3MB free
> > space.
> > nothing show low space toast. 
> > if remove external SD, you can see toast.
> > 
> > (In reply to Jason Smith [:jsmith] from comment #1)
> > > This is definitely a non-blocker - we already show to the user a system-wise
> > > toast when internal storage gets low and then persist a notification
> > > indicating storage is low until 10 MB of free internal space is hit. Having
> > > an additional check in camera to improve error handling is a good idea, but
> > > a nice to have if a system-wise indicator is already present.
> 
> That's not what I'm seeing. I just tested this on a 8/13 Comm RIL build and
> tried to write past the 5 MB limit down 3 MB to see if we block the write.
> We did successfully block the write, which means our low storage detection
> is working. My testing I did was with a sdcard present as well.

Note though that I did this testing with a leo device with the app install workflow. So I might need to also check the non-app install workflow to see if that still works.

Comment 5

[Jason Smith [:jsmith]]

Just confirmed that we're definitely getting the low storage notification toast on a leo 8/13 comm ril by pushing data directly below the 5 MB limit. We also are showing the persistent low storage notification as well.

Comment 6

[Jason Smith [:jsmith]]

Attachment: Low Storage Notification

Comment 7

[leo.bugzilla.gecko]

I also test 8/13 build version. 
Internal Storage is Full.SD card storage is over 10MB free space.
Default media location is Internal. 
issue should be device have 2 sd card.

If remove SD Card. device show low space.

Comment 8

[Andrew Overholt [:overholt]]

Could there be a commit that's affecting this behaviour in Jason's build that's not in leo's build?

Comment 9

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #8)
> Could there be a commit that's affecting this behaviour in Jason's build
> that's not in leo's build?

One idea I haven't tried yet was to test this directly on a leo partner build. The testing I did above was on a comm RIL build we generate.

There's definitely a possibility this could be a POVB issue if this happens on a partner build, but not our builds.

Comment 10

[jongsoo.oh]

Low Storage Notification is displayed when the application storage is remained less than 5 MB.  
The noti. is not shown to sdcard storage empty in leo.

Comment 11

[Jason Smith [:jsmith]]

(In reply to jongsoo.oh from comment #10)
> Low Storage Notification is displayed when the application storage is
> remained less than 5 MB.  
> The noti. is not shown to sdcard storage empty in leo.

Correct. We only show the low storage notification with application storage (i.e /data). There isn't an implementation right now for reporting low storage with the sdcard.

Comment 12

[Andrew Overholt [:overholt]]

1.1 doesn't have support for low storage space notifications on SD cards.  Since this is a new feature request, moving to koi?

Comment 13

[leo.bugzilla.gecko]

If device don't have the internal free space.
Should not go into the camera menu. and Photos should not be taken. 
Because device have no memory space.

Comment 14

[Andrew Overholt [:overholt]]

(In reply to leo.bugzilla.gecko from comment #13)
> If device don't have the internal free space.
> Should not go into the camera menu. and Photos should not be taken. 
> Because device have no memory space.

Sure, but users won't be able to get into this state without pushing things via adb, right?

Comment 15

[Dave Hylands [:dhylands]]

Andrew: If by "that state" you mean low internal storage space, then you can get into that state many different ways.

Internal storage (somewhere until /data) is where the indexed db files live, and where new apps get installed, so you can fill it up without using adb.

Comment 16

[Andrew Overholt [:overholt]]

Sorry, I missed Dave's comment here.  While quite a valid concern, comment 13 is a new feature request, right, Dave?

Comment 17

[Dave Hylands [:dhylands]]

I believe so. Currently, we just prevent IndexedDB writes from occuring and display a dialog to the user to tell them that internal storage is low. It would obviously be better if apps detected the low memory condition and did something intelligent. I think that this probably needs to be addressed on an app by app basis.

Comment 18

[Fernando Jiménez Moreno [:ferjm]]

Quickly reading this thread it seems that you might be having some misunderstanding about the storage type. You mentioned "internal storage" referring to both the /data partition and the internal sdcard.

In any case, IMHO for things like pictures, music and videos is probably the app the one responsible for checking if there is enough free space before trying to store a file and the one responsible for warning the user in case of a low storage condition. We already provide the tools to do that, check [1].

On the other hand, we should probably want to expose a way for apps to check if the device is under a low storage condition (/data partition between 5Mb and 10Mb of free space) which causes indexedDb, localStorage and offline cache to become read only. Currently, we notify the user, but there is not a good way for apps to check this.

[1] https://mxr.mozilla.org/mozilla-central/source/dom/interfaces/devicestorage/nsIDOMDeviceStorage.idl#25

Comment 19

[Peter Dolanjski [:pdol]]

(In reply to Fernando Jiménez Moreno [:ferjm] (not reading bugmail until 14th October) from comment #18)
> On the other hand, we should probably want to expose a way for apps to check
> if the device is under a low storage condition (/data partition between 5Mb
> and 10Mb of free space) which causes indexedDb, localStorage and offline
> cache to become read only. Currently, we notify the user, but there is not a
> good way for apps to check this.

Fernando, is there another bug for what you are describing?
I'd like to turn this bug into one described by comment 13, whereby the user is prevented from taking pictures/video since the storage is full.

Comment 20

[Fernando Jiménez Moreno [:ferjm]]

Actually, I was not correct in that comment (not sure why I said that...). We do have a way for apps to check the free space in the /data partition. Just like apps can check the free space for pictures, music or videos, apps can check the free space for the 'apps' device storage to check the free space in the /data partition:

navigator.getDeviceStorage('apps').freeSpace();

Comment 21

[Jason Smith [:jsmith]]

Not blocking - too late to land this feature for 1.2

Comment 22

[Peter Dolanjski [:pdol]]

(In reply to Fernando Jiménez Moreno [:ferjm] (use needinfo instead of CC, please) from comment #20)
> We do have a way for apps to check the free space in the /data partition.
> Just like apps can check the free space for pictures, music or videos, apps
> can check the free space for the 'apps' device storage to check the free
> space in the /data partition:
> 
> navigator.getDeviceStorage('apps').freeSpace();

If this is the case, it sounds like this should be a Camera specific bug to prevent users from taking photos if there is no free space.  Please correct me if I'm wrong.

Comment 23

[Sri Kasetti]

Added to the media backlog

Comment 24

[Diego Marcos [:dmarcos]]

Attachment: noSpaceMessage.JPG

This feature is already available in camera