1004749 - [OTA] No Update UI message to the end user that there's no space left if the data/local and sdcard are filled

Status: RESOLVED WONTFIX

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: logcat.txt

1. flash with a build earlier than today
2. fill the sdcard with junk 
3. fill the data/local partition with junk
4. set the channel to the appropriate update channel (tarako/1.3.0t/nightly)
5. reboot and turn on wifi
6. force system update
7. try to download the system update

Expected: UI error message stating that the device partiions are filled and there needs to be some space
Actual: no error message to the end user, if you look in logcat you can see any errors; UI will continue to show the notification of 1 update available (later/download)

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Gaia      e87f8a69a2bad4a1361529b180a4e015fc78fbf1
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/1350653d7e40
BuildID   20140422014001
Version   28.1
ro.build.version.incremental=eng.cltbld.20140422.051457
ro.build.date=Tue Apr 22 05:15:05 EDT 2014
Tarako


I'm pretty sure this isn't tarako specific, but we should test regardless...

Comment 2

[Jason Smith [:jsmith]]

There is a low space warning implemented in Firefox OS for /data/local, but this will only work on partner builds that have fanotify enabled. Past releases do have this functionality working, so that means we need to check if fanotify is enabled.

Fabrice - Can you remind me how we check if a build has fanotify enabled?

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Oh I wonder if we turned that off for memory reasons?

Comment 4

[Jason Smith [:jsmith]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #3)
> Oh I wonder if we turned that off for memory reasons?

That better not be true. There's a certification requirement around this.

Comment 5

[[:fabrice] Fabrice Desré]

We have fanotify enabled on tarako. Note that it triggers UI warning only when we get below 5MB of free space in /data while updates will refuse to install if we don't have enough space for the update, which is usually around 50MB. So I'm not surprised that we don't get a low-space warning but still can't download updates.

Comment 6

[Jason Smith [:jsmith]]

Ah right. The test case here isn't exactly realistic. This needs to be retested as follows:

- Start from a state where you are just above the low storage threshold (above 5 MB in /data)
- Do an operation on the phone that would cause storage to go below 5 MB for /data
- Verify that a low storage toast appears & a persistent notification shows up in the tray

sdcard low storage doesn't have any notification, which I think is tracked in a separate bug somewhere.

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Actually it's very realistic.  You load a couple of web apps to fill the data/local partition and fill a 2 gig SD card with videos, music and pictures.

Even if you do get the low storage notification, which I didn't; the storage notification is a notification that goes away.  If we're downloading an update, we should notify the user of that there is no space at that time.  Both iPhone and Android does this.  We don't.

Comment 8

[Jason Smith [:jsmith]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #7)
> Actually it's very realistic.  You load a couple of web apps to fill the
> data/local partition and fill a 2 gig SD card with videos, music and
> pictures.

Not in the way it was tested above. That's directly modification of the file system via adb, which isn't what a user can do a production phone with /data. If it's was done via a user workflow, then the low storage UI & fanotify would be triggered.

Comment 9

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Android and iphone have specific messages stating you need x amount of space in order to update the phone to the latest OS.  We don't provide this specific message for updating.

Comment 10

[[:fabrice] Fabrice Desré]

Right, but that's not tarako specific?

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Fabrice, I don't think so.

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Also to note, the update doesn't bother trying to place the file and then running out of space.  the space left could be 10 megs and the update could fail to give the notification.

Comment 13

[Jason Smith [:jsmith]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #9)
> Android and iphone have specific messages stating you need x amount of space
> in order to update the phone to the latest OS.  We don't provide this
> specific message for updating.

To my understanding, that is indicated the UI. The prompt for updating includes an indication of how much space is needed for the system update.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #12)
> Also to note, the update doesn't bother trying to place the file and then
> running out of space.  the space left could be 10 megs and the update could
> fail to give the notification.

In that case, if the OTA update is being written to a core piece of the phone, then there should be a toast fired indicating you've hit low storage & the update should fail. There wouldn't be a persistent notification for low storage in this case though because you will return to a state that has write access.

Question - Where is the OTA update files getting written to? Internal or external storage?

Comment 15

[[:fabrice] Fabrice Desré]

If we don't have enough space, we just don't try to download the update, so we don't use any space. See https://mxr.mozilla.org/mozilla-central/source/b2g/components/DirectoryProvider.js#203

Comment 16

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

(In reply to Jason Smith [:jsmith] from comment #14)
> Question - Where is the OTA update files getting written to? Internal or
> external storage?

See Fabrice's comment.  It checks the data partition for space, then it checks the sdcard for space.  So it depends on which has space.

There is no notification that occurs if the size of the file is > than the notification point since the code checks before writing.

Comment 17

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

To clarify the last comment : if the file size is > the space left and the file size is > the notification limit.

Comment 18

[Jason Smith [:jsmith]]

Got this clarified in IRC - the problem here reinforces the existing bad UX we have for errors in system updates. So what's being here is to fix up the error handling for when low storage is hit.

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

James, just a fyi that we don't have a UI that supports saying how much space we have left on the device.

Comment 20

[James Zhang (Spreadtrum)]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #19)
> James, just a fyi that we don't have a UI that supports saying how much
> space we have left on the device.

I think we should support this feature -- if there's no space, we can clear application cache or install application to sdcard.
The first tarako software version will deliver to our customer 5/10, they must have this requirement.

Comment 21

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

noming based on comment 20.

Comment 22

[Jason Smith [:jsmith]]

We should not be adding feature requests at this point in the release, especially if it's been around in past releases that we've already shipped. A feature like this also poses string risk, which is going to risk shipping an unlocalized string. This should not block.

Comment 23

[[:fabrice] Fabrice Desré]

I still feel we should display some informative message to the user in these situations. Eg. "Please free up space on your sdcard to be able to download updates". That could be a notification.

James, feel free to assign to someone on your side. If not I'll find someone.

Comment 24

[James Zhang (Spreadtrum)]

Loop Arvin, can you provide patch?

Comment 25

[Joe Cheng [:jcheng] (please needinfo)]

ni? Steven to understand partner's update flow

Comment 26

[Steven Yang [:styang]]

One of the OEM will still host FOTA server, it's good to have this. thanks.

Comment 27

[bhavana bajaj [:bajaj]]

We will have to discuss what the impact here is on l10n, but the reason for blocking this in triage was to give a  good update experience to the user

Comment 28

[Jason Smith [:jsmith]]

(In reply to bhavana bajaj [:bajaj] from comment #27)
> We will have to discuss what the impact here is on l10n, but the reason for
> blocking this in triage was to give a  good update experience to the user

I don't think that's a strong enough case to block here. We've shipped with this problem since 1.0. Why is this needed now?

Comment 29

[[:fabrice] Fabrice Desré]

(In reply to Jason Smith [:jsmith] from comment #28)

> I don't think that's a strong enough case to block here. We've shipped with
> this problem since 1.0. Why is this needed now?

Because this happens way more often on Tarako for two reasons:
- limited ROM storage.
- no sdcard by default to fallback on bundled by carriers.

Comment 30

[Jason Smith [:jsmith]]

Fair enough.

Comment 31

[Gregor Wagner [:gwagner]]

Arvin, any update here?

Comment 32

[helloarvin]

Hi Gregor,
  
  Sorry to reply late and please shiwei help to update state here.
  Thanks.

Comment 33

[Gregor Wagner [:gwagner]]

Shiwei, what's the status with this bug? Are you working on it?

Comment 34

[Steven Yang [:styang]]

James, how's the status in your side?

Comment 35

[Shiwei Zhang]

I am sorry for took so long to reply，considering Fabrice's comment 15, getUpdateDir is a gecko function, but UI message should be prompt in gaia. that should take time to find out how to call getUpdateDir from gaia.

Comment 36

[James Zhang (Spreadtrum)]

Fabrice and Steven, can you help here?
Shiwei, you can ask Jesse for help.

Comment 37

[Kai-Zhen Li [:seinlin][:kli]]

Shiwei, 

As I know, now there are the following update status and messages.
---------
status                 - messages
---------
checking-for-update    - Checking for updates…
no-updates             - No updates were found
retry-when-online      - Network is offline. Will check again when the network is online.
already-latest-version - This is already the latest version of {{brandShortName}}
check-error            - There was an error when checking for updates.
---------

All other status not in the above list will be displayed as 'check-error'.

The update status is updated in UpdatePrompt.js such as
http://dxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt.js#92

The message of each status is defined in each l10n properties, for en-US such as.
gaia/apps/settings/locales/settings.en-US.properties


To meet the requirement of the bug, I think you'll need to do
- add a new status for update
- add a new message for the new added status. (Need to add in all l10n repo.)

Comment 38

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: error message when available space is not enough

I have a test with today build. 

When the available space is not enough, I can see the error message "There was an error when checking for updates" as comment 37.

But the error message is displayed about 30 seconds after download is pressed, and the messages is displayed only about 1 second.

I think the message should be displayed asap and keep until user touch to clear it.


When download is pressed, I can see the below log which shows no space is available to download update file.
---
I/Gecko   (   85): DirectoryProvider: Error: No volume found with 82571145.3 bytes for downloading update Firefox 33
I/Gecko   (   85): UpdatePrompt: Error downloading update Firefox 33: -2142109681
I/Gecko   (   85): UpdatePrompt: Update error, state: downloading, errorCode: -2142109681
I/Gecko   (   85): UpdatePrompt: Setting gecko.updateStatus: file-too-big

Comment 39

[Shiwei Zhang]

(In reply to Kai-Zhen Li from comment #38)
> Created attachment 8430641 [details]
> error message when available space is not enough
> 
> I have a test with today build. 
> 
> When the available space is not enough, I can see the error message "There
> was an error when checking for updates" as comment 37.
> 
> But the error message is displayed about 30 seconds after download is
> pressed, and the messages is displayed only about 1 second.
> 
> I think the message should be displayed asap and keep until user touch to
> clear it.
> 
> 
> When download is pressed, I can see the below log which shows no space is
> available to download update file.
> ---
> I/Gecko   (   85): DirectoryProvider: Error: No volume found with 82571145.3
> bytes for downloading update Firefox 33
> I/Gecko   (   85): UpdatePrompt: Error downloading update Firefox 33:
> -2142109681
> I/Gecko   (   85): UpdatePrompt: Update error, state: downloading,
> errorCode: -2142109681
> I/Gecko   (   85): UpdatePrompt: Setting gecko.updateStatus: file-too-big

Thanks a lot, Kai-Zhen, I'll have a try.

Comment 40

[Shiwei Zhang]

(In reply to Kai-Zhen Li from comment #38)
... ...
> I think the message should be displayed asap and keep until user touch to
> clear it.
> 
> 
> When download is pressed, I can see the below log which shows no space is
> available to download update file.
> ---
> I/Gecko   (   85): DirectoryProvider: Error: No volume found with 82571145.3
> bytes for downloading update Firefox 33
> I/Gecko   (   85): UpdatePrompt: Error downloading update Firefox 33:
> -2142109681
> I/Gecko   (   85): UpdatePrompt: Update error, state: downloading,
> errorCode: -2142109681
> I/Gecko   (   85): UpdatePrompt: Setting gecko.updateStatus: file-too-big

Kai-Zhen，I have network problems downloading the system update, the log shows 

E/GeckoConsole( 4161): AUS:SVC Checker:onLoad - request completed downloading document
E/GeckoConsole( 4161): AUS:SVC Checker:_updates get - unexpected node name!
E/GeckoConsole( 4161): AUS:SVC Checker:onLoad - there was a problem checking for updates. Exception: Error: Unexpected node name, expected: updates, got: parsererror
E/GeckoConsole( 4161): AUS:SVC Checker:onLoad - request.status: 404
E/GeckoConsole( 4161): AUS:SVC getStatusTextFromCode - transfer error: Update XML file not found (404), code: 404

but I have an idea to handle the issue. Considering your suggestions in comment 37, since gecko.updateStatus is accessible to gaia, we can trigger the UI refering to gecko.updateStatus. That is, if file-too-big appeares in gecko.updateStatus UI message could be prompt. I'll test out this idea by experiment.

Do you agree with me?

Comment 41

[Shiwei Zhang]

Attachment: out_of_memory_notification.patch

Kai-Zhen，As described in my last comment, network problems prevents me to do system update, could you help me to verify this patch. 
it's base on commit fe4b20403e6fee8b54a69aa872bfb35cbc9af651 
B2G/gaia 
Author: steveck-chung <[e-mail]>
Date:   Tue Jun 3 17:46:54 2014 +0800

I am very grateful for your kindly support and help.

Comment 42

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: update log

I verified with PAC version FIREFOXOS_V1.3_W14.22.5_SP6821, update file can be downloaded and updated successfully. As attached log.

Comment 43

[Shiwei Zhang]

(In reply to Kai-Zhen Li from comment #42)

Kai-Zhen， I tried several times but all failed, the log shows

E/GeckoConsole(   84): AUS:SVC Checker: checkForUpdates, force: true
E/GeckoConsole(   84): AUS:SVC Checker:getUpdateURL - update URL: http://update.boot2gecko.org/release-spreadtrum/update.xml?force=1
E/GeckoConsole(   84): AUS:SVC gCanCheckForUpdates - able to check for updates
E/GeckoConsole(   84): AUS:SVC Checker:checkForUpdates - sending request to: http://update.boot2gecko.org/release-spreadtrum/update.xml?force=1
E/GeckoConsole(   84): [JavaScript Error: "syntax error" {file: "http://update.boot2gecko.org/release-spreadtrum/update.xml?force=1" line: 1 column: 50 source: "<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">"}]

I think there is nothing in the update URL: http://update.boot2gecko.org/release-spreadtrum/update.xml?force=1 , because it is not accessable to ordinary browser either.

Maybe I should change another update URL by modify pref("app.update.url") in B2G/gecko/b2g/app/b2g.js.
could you tell me an accessable one.

Thanks a lot.

Comment 44

[Kai-Zhen Li [:seinlin][:kli]]

(In reply to Shiwei Zhang from comment #43)
> 
> I think there is nothing in the update URL:
> http://update.boot2gecko.org/release-spreadtrum/update.xml?force=1 , because
> it is not accessable to ordinary browser either.
> 
> Maybe I should change another update URL by modify pref("app.update.url") in
> B2G/gecko/b2g/app/b2g.js.
> could you tell me an accessable one.
> 
> Thanks a lot.

Yes, you'll need a valid update url to test the update. 
Or you can also use nightly channel for testing.
Add the following line into "device/sprd/sp6821a_gonk/dev-pref.js", and then push it to /system/b2g/defaults/pref/dev-pref.js. Restart the device and try again.
--
pref("app.update.url", "http://update.boot2gecko.org/tarako/1.3.0t/nightly/update.xml");
--

Comment 45

[Faramarz Rashed]

Last comment here is from 3days ago. What's the update? This is a blocker!

Comment 46

[Shiwei Zhang]

(In reply to Kai-Zhen Li from comment #44)
Thanks a lot for your advise. I will try it out as soon as I could.

Comment 47

[Shiwei Zhang]

Attachment: UI_message_for_no_space_OTA.patch

Kai-Zhen, Thanks again for your kindly help, I think this patch can handle the issue. please gave a review.

Comment 48

[Kai-Zhen Li [:seinlin][:kli]]

(In reply to Shiwei Zhang from comment #47)
> Created attachment 8437512 [details] [diff] [review]
> UI_message_for_no_space_OTA.patch
> 
> Kai-Zhen, Thanks again for your kindly help, I think this patch can handle
> the issue. please gave a review.

With the attached patch, I can't see the message when update status is updated to file-too-big.
Can you attach the screenshot and STR?

--
I/Gecko   (  689): *** AUS:SVC Creating Downloader
E/GeckoConsole(  689): AUS:SVC Creating Downloader
I/Gecko   (  689): *** AUS:SVC UpdateService:_downloadUpdate
E/GeckoConsole(  689): AUS:SVC UpdateService:_downloadUpdate
I/Gecko   (  689): *** AUS:SVC readStatusFile - status: null, path: /data/local/updates/0/update.status
E/GeckoConsole(  689): AUS:SVC readStatusFile - status: null, path: /data/local/updates/0/update.status
I/Gecko   (  689): *** AUS:SVC Downloader:_selectPatch - found existing patch with state: null
E/GeckoConsole(  689): AUS:SVC Downloader:_selectPatch - found existing patch with state: null
I/Gecko   (  689): *** AUS:SVC Downloader:_selectPatch - failed to apply complete patch!
E/GeckoConsole(  689): AUS:SVC Downloader:_selectPatch - failed to apply complete patch!
I/Gecko   (  689): *** AUS:SVC Downloader:downloadUpdate - no patch to download
E/GeckoConsole(  689): AUS:SVC Downloader:downloadUpdate - no patch to download
I/Gecko   (  689): *** AUS:SVC readStatusFile - status: null, path: /data/local/updates/0/update.status
E/GeckoConsole(  689): AUS:SVC readStatusFile - status: null, path: /data/local/updates/0/update.status
I/Gecko   (  689): UpdatePrompt: Error downloading update Firefox 28.1: -2142109681
I/Gecko   (  689): UpdatePrompt: Update error, state: null, errorCode: -2142109681
I/Gecko   (  689): UpdatePrompt: Setting gecko.updateStatus: file-too-big
I/GeckoDump(  689): XXX FIXME : Got a mozContentEvent: update-available-result

Comment 49

[Kai-Zhen Li [:seinlin][:kli]]

I also notice that there is js error, when udpate status is set to file-too-big.

E/GeckoConsole(  843): Content JS LOG at app://settings.gaiamobile.org/js/about.js:101 in onUpdateStatus: file-too-big
E/GeckoConsole(  843): Content JS ERROR at app://settings.gaiamobile.org/js/about.js:112 in onUpdateStatus: Error checking for system update: file-too-big
E/GeckoConsole(  843): Content JS LOG at app://settings.gaiamobile.org/js/about.js:101 in onUpdateStatus: file-too-big
E/GeckoConsole(  843): Content JS ERROR at app://settings.gaiamobile.org/js/about.js:112 in onUpdateStatus: Error checking for system update: file-too-big

Comment 50

[Shiwei Zhang]

Attachment: 20140611_181649.jpg

(In reply to Kai-Zhen Li from comment #49)
> I also notice that there is js error, when udpate status is set to
> file-too-big.
> 
> E/GeckoConsole(  843): Content JS LOG at
> app://settings.gaiamobile.org/js/about.js:101 in onUpdateStatus: file-too-big
> E/GeckoConsole(  843): Content JS ERROR at
> app://settings.gaiamobile.org/js/about.js:112 in onUpdateStatus: Error
> checking for system update: file-too-big
> E/GeckoConsole(  843): Content JS LOG at
> app://settings.gaiamobile.org/js/about.js:101 in onUpdateStatus: file-too-big
> E/GeckoConsole(  843): Content JS ERROR at
> app://settings.gaiamobile.org/js/about.js:112 in onUpdateStatus: Error
> checking for system update: file-too-big

Kai-Zhen，the log seems right. I had one more try，the patch can works normally，please see screenshots in attachment.

Comment 51

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: 2014-06-11-19-31-07.png

Finally, I can see the message as your screenshot. But I think this solution is not good enough because of the following 2 reasons.
1. need to keep setting app on foreground.
2. when the message is displayed, it is behind the system notification (as attached file.)

Can you try again with back to homescreen before start to download?

Comment 52

[Joe Cheng [:jcheng] (please needinfo)]

i'd like to have UX review here as well. Ni? Mike

Comment 53

[Shiwei Zhang]

(In reply to Kai-Zhen Li from comment #51)
> Created attachment 8438374 [details]
> 2014-06-11-19-31-07.png
> 
> Finally, I can see the message as your screenshot. But I think this solution
> is not good enough because of the following 2 reasons.
> 1. need to keep setting app on foreground.
> 2. when the message is displayed, it is behind the system notification (as
> attached file.)
> 
> Can you try again with back to homescreen before start to download?

 You means this procedure could separate from settings and just connect with download things? My main focuse is offering an UI message for update checking when memory insufficient. Maybe moving this procedure into system app will meet the two points in your comment.

If you agree with me ,I'll have a try.

Comment 54

[Gregor Wagner [:gwagner]]

You have to officially ask for review on the patch.

Comment 55

[Shiwei Zhang]

Gregor, I think Kai-Zhen's suggestion reasonable. if time is not urgency I will have a try and give a more appropriate patch.

Comment 56

[Kai-Zhen Li [:seinlin][:kli]]

(In reply to Shiwei Zhang from comment #53)
> (In reply to Kai-Zhen Li from comment #51)
> > Created attachment 8438374 [details]
> > 2014-06-11-19-31-07.png
> > 
> > Finally, I can see the message as your screenshot. But I think this solution
> > is not good enough because of the following 2 reasons.
> > 1. need to keep setting app on foreground.
> > 2. when the message is displayed, it is behind the system notification (as
> > attached file.)
> > 
> > Can you try again with back to homescreen before start to download?
> 
>  You means this procedure could separate from settings and just connect with
> download things? My main focuse is offering an UI message for update
> checking when memory insufficient. Maybe moving this procedure into system
> app will meet the two points in your comment.
> 
> If you agree with me ,I'll have a try.

Yes, user can be notified to there is update to download from notification bar. No need to open setting App. 
- I think you can follow comment 37 and comment 38 to add a new message for this status. 
- Display the message until user close it, not only 1 second.

Comment 57

[Shiwei Zhang]

Attachment: UI_for_out_of_memory.patch

This patch is more reasonable，could you give a review, thanks.

Comment 58

[Shiwei Zhang]

Attachment: UI_for_out_of_memory.patch

This patch is more reasonable，could you give a review, thanks. + review ?

Comment 59

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8439675 [details] [diff] [review]
UI_for_out_of_memory.patch

This looks incomplete.

-- If there will be a new value 'file-too-big' from Gecko, Settings app need some modification too.
-- testObject shouldn't be named testObject. It should not contain hardcoded strings either. Please move the English strings to localization file so people can localize them.
-- English strings need UI review. I am fairly certain the title shouldn't be the scary "OUT OF MEMORY".

Comment 60

[Stephany Wilkes]

Comment on attachment 8439675 [details] [diff] [review]
UI_for_out_of_memory.patch

Flagging Tif for ui-review? to try to get this done today, in PST time zone.

Comment 61

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8439675 [details] [diff] [review]
UI_for_out_of_memory.patch

As mentioned in the previous comment, we definitely need to be able to localize our strings.

Additionally, we need to re-write the string, but before the correction can be provided, I need a little more info. 

To be clear, this string is only used in the instance of trying to download a system update and there isn't enough storage space to do so - is that correct?

If so, is it possible to be more specific with how much space it needed? Otherwise I can see one of two things happening - the user will try deleting a couple of things and re-trying the download several times until they  give up in exasperation. Or the user goes on a massive purge and deletes every excess thing possible when they really only needed to remove a few files.

Finally, I took a look at the screenshot, and assuming that is what the patch is doing, the cancel button should be at the bottom of the screen. This dialogue should be using our standard dialogue Building Block and thus be full screen. Also, the button should be "ok" instead of "cancel".

NI'ing Francis as I believe he has written a "low storage" string in the past.

Thanks guys!!

Comment 62

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: system_show_file-too-big.diff

Shiwei, This is a draft of my idea in comment 37 and comment 38. But there is still some works need to be done as in patch. Can you try this patch and continue to finish it if you think it is reasonable?

Comment 63

[Shiwei Zhang]

Considering Tiffanie and Tim's suggestion, we need to re-write the English strings and move them to localization files, and the dialogue need to tell how much space it needed and how much space is available，am I right？

Comment 64

[Shiwei Zhang]

Thanks Kai-Zhen, I am afraid we need considering the status of storage space and how much space the system update needed in update_manager.js. Since I have so many things to do in these two days, I will to try this patch as soon as I can.

Comment 65

[Shiwei Zhang]

Attachment: UI_for_updates_no_space.patch

Maybe this patch could fit all above requirements，could you give a review, thanks.

Comment 66

[Tiffanie Shakespeare [:tif]]

Hi guys! It's hard for me to repro with the patch on my device - can you confirm that the dialogue is conforming to our building blocks? (i.e. full screen and button resides at the bottom)

Looking at the patch code it appears the string being displayed is:
Title: Insufficient space
Body: You have only <x> MB of free space on your current storage.
Button: Close

Assuming this is strictly for downloading updates (see my questions in comment 61) I would say:
Title: Cannot Download Update
Body: This update requires <x> MB of additional storage to download.\nPlease delete files to free space before trying to update again.
Button: OK

*note that the MB is the subtraction of what is free minus the size of the update. Also note the line break between the two sentences.

LMK if this is not the case and I can tweak the string. Thanks!

Comment 67

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8443353 [details] [diff] [review]
UI_for_updates_no_space.patch

>diff --git a/apps/system/locales/system.en-US.properties b/apps/system/locales/system.en-US.properties
>index 4d1ce28..58d3877 100644
>--- a/apps/system/locales/system.en-US.properties
>+++ b/apps/system/locales/system.en-US.properties
>@@ -101,6 +101,9 @@ continue=Continue
> stopDownloading=Stop downloading {{ app }}?
> app-download-can-be-restarted=The download can be restarted later.
> app-download-stop-button=Stop Download
>+updates-download-out-of-memory1=Insufficient space :  You have only 
Should be "Insufficient space:  You have only "

>+updates-download-out-of-memory3=MB of free space on your current storage.
>+updates-download-cancel-button=Cancel Download
> not-enough-space=Not enough space
> not-enough-space-message=There is not enough space to install this app. Free up space by deleting old apps or media, and try installing again from the original source.
> app-install-success={{ appName }} installed

Comment 68

[Brogan Zumwalt [Inactive]]

Issue is occurring on 2.0 Flame during OTA from 20140623000201 to 20140624000201

With 44.2 MB free in /Data and 4 MB free on SD Card, selecting update in Notification tray does not download and no warning indicating there is no space available.

Environmental Variables:
Device: Flame 2.0
Build ID: 20140623000201
Gaia: 729f214b887ce8efe7d870145d31acb2c6427817
Gecko: 117ba3eda4d2
Version: 32.0a2 (2.0) 
Firmware Version: v121-2

Comment 69

[Shiwei Zhang]

Attachment: UI_for_updates_no_space_m.patch

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #67)
Thanks, This is a modified patch according to your opinion.

Comment 70

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8447977 [details] [diff] [review]
UI_for_updates_no_space_m.patch

I think the both lines below should be "Insufficient space: You have only ", one space after colon and no space before it.

>+          <span data-l10n-id="updates-download-out-of-memory1">Insufficient space:  You have only </span>

>+updates-download-out-of-memory1=Insufficient space :  You have only 


I don't think I really need to review this again so I am setting r+ now and please correct it before landing. Thanks.

Comment 71

[Tiffanie Shakespeare [:tif]]

Please see the string we should be using in my comment #66. We should not be using the "Insufficient space" string that I am seeing in the code and above comment. Thanks guys!

(In reply to Tiffanie Shakespeare from comment #66)

> Title: Cannot Download Update
> Body: This update requires <x> MB of additional storage to download.\nPlease
> delete files to free space before trying to update again.
> Button: OK
> 
> *note that the MB is the subtraction of what is free minus the size of the
> update. Also note the line break between the two sentences.

Comment 72

[Stephany Wilkes]

All, please work hard to get the correct strings in. We are now well past string freeze but were not when Tiffanie posted her initial comments about this on June 13.

Comment 73

[Stephany Wilkes]

Comment on attachment 8447977 [details] [diff] [review]
UI_for_updates_no_space_m.patch

Flagging Tif, as she should have been flagged earlier on ui-review? on this patch. This would have enabled her to minus it until the correct strings were done.

Comment 74

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8447977 [details] [diff] [review]
UI_for_updates_no_space_m.patch

From what I can see in the code above we are using the incorrect string. See comment #71

Comment 75

[Gregor Wagner [:gwagner]]

Any updates here?

Comment 76

[Stephany Wilkes]

Based on ui-review- from Tif, it looks like a new patch should be submitted correcting the string. Setting ni? to Tim to see if that is coming or if he has insight here.

Comment 77

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8447977 [details] [diff] [review]
UI_for_updates_no_space_m.patch

Retracting r+ based on UI review.

Shi-Wei, would you please update the strings and submit a new patch?

Comment 78

[Shiwei Zhang]

Attachment: UI_for_no_space_update.patch

I think the state that This update requires <x> MB of additional storage to download in Comment 66 is not perfect in that case, because we will be informed the sizes of update files before submit download. Maybe telling how much space the device currently has is more useful.

This patch is an appropriate one，please give a review,thanks.

Comment 79

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8453574 [details] [diff] [review]
UI_for_no_space_update.patch

Please remove console.log() that can show up during normal situation.

(i.e. remove the ones in .success() but keep the one in .onerror() )

Comment 80

[Shiwei Zhang]

Attachment: UI_for_no_space_update_m.patch

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #79)
> Comment on attachment 8453574 [details] [diff] [review]
> UI_for_no_space_update.patch
> 
> Please remove console.log() that can show up during normal situation.
> 
> (i.e. remove the ones in .success() but keep the one in .onerror() )

console.log() removed.

Comment 81

[Tiffanie Shakespeare [:tif]]

If I follow you correctly, you're saying it isn't possible to know the size of the update therefore we will not be able to tell the user how much MB they need to free up?

Is this correct?

(In reply to Shiwei Zhang from comment #78)
> Created attachment 8453574 [details] [diff] [review]
> UI_for_no_space_update.patch
> 
> I think the state that This update requires <x> MB of additional storage to
> download in Comment 66 is not perfect in that case, because we will be
> informed the sizes of update files before submit download. Maybe telling how
> much space the device currently has is more useful.
> 
> This patch is an appropriate one，please give a review,thanks.

Comment 82

[Stephany Wilkes]

What's going on with the ui-review flag use here? I see Tif gave a ui-review- and it was removed, but has not been reset to ui-review? with updates. 

This still needs to get ui-review+ to move forward, even if it has review+.

Please set these flags accordingly. Thanks!

Comment 83

[Stephany Wilkes]

It also looks like new strings were submitted as part of this patch. Adding the late-l10n flag for this reason. Please remove if that's not correct.

Comment 84

[Shiwei Zhang]

Attachment: IMG_0006.jpg

(In reply to Tiffanie Shakespeare from comment #81)
> If I follow you correctly, you're saying it isn't possible to know the size
> of the update therefore we will not be able to tell the user how much MB
> they need to free up?
> 
> Is this correct?

No, I mean we need not tell how much MB to free up, the user can see how much MB the update need before submit the download isn't it ? please see this snapshot.
Only he need to know is how much space left on the device.

Comment 85

[Tiffanie Shakespeare [:tif]]

Ahh yes, I see in image_006 that it lists the file size. Perfect! We do know the update size and can thus inform the user of how much space they need to free.

The reason why we want to provide this info to the reason is two fold:
1. Why force the user to do math when they don't have to? Software can quickly and accurately do this calculation for people.
2. The update size is on a different screen than this error message. So not only are we asking people to do math, but we are also requiring them to remember the size of the update; preemptively at that since the user has no knowledge before tapping download that they may need to remember this number.

So instead, we can go a more user-centered route and do all the remembering and calculations for the user. This is why I'm so intent on telling the user how much space they need to clear.

What would be an even better solution, which was not apparent to me until just now, is to skip the error message altogether and right there on that screen, image_006, before the user even taps download, tell them they don't have enough space to download the update and tell them how much more space they need. But I get that time is limited and we've already gone the error message route so this idea may have to be a follow-up bug.

Which in that case, we do need to tell the user how much space is required to be freed up. 

Thanks Shiwei and hopefully that decision makes more sense now that you have the rationale and have put yourself in the user's position.

Comment 86

[Francesco Lodolo [:flod]]

(In reply to Stephany Wilkes from comment #83)
> It also looks like new strings were submitted as part of this patch. Adding
> the late-l10n flag for this reason. Please remove if that's not correct.

Which version is this bug targeting?

> +updates-download-out-of-memory1=There is only 
> +updates-download-out-of-memory3=MB free space on your current storage. Please delete files to free space before trying to update again.

You're concatenating strings. Don't do that, use placeholders or variables instead for the number. E.g: 

updates-download-out-of-memory= There is only {{size}} MB free space on your current storage. Please delete files to free space before trying to update again.

Several locales will have to turn this sentence around to avoid issues with plural. Also not sure what's the reason to tell users how much space is available, when we don't tell them how much they need to free.

Comment 87

[Tiffanie Shakespeare [:tif]]

Hey Francesco! If you look at comment 66, you'll see the string should be: 

Title: Cannot Download Update
Body: This update requires <x> MB of additional storage to download.\nPlease delete files to free space before trying to update again.
Button: OK

Thanks!

(In reply to Francesco Lodolo [:flod] from comment #86)

> updates-download-out-of-memory= There is only {{size}} MB free space on your
> current storage. Please delete files to free space before trying to update
> again.
> 
> Several locales will have to turn this sentence around to avoid issues with
> plural. Also not sure what's the reason to tell users how much space is
> available, when we don't tell them how much they need to free.

Comment 88

[Shiwei Zhang]

(In reply to Tiffanie Shakespeare from comment #85)
> Ahh yes, I see in image_006 that it lists the file size. Perfect! We do know
> the update size and can thus inform the user of how much space they need to
> free.
> 
> The reason why we want to provide this info to the reason is two fold:
> 1. Why force the user to do math when they don't have to? Software can
> quickly and accurately do this calculation for people.
> 2. The update size is on a different screen than this error message. So not
> only are we asking people to do math, but we are also requiring them to
> remember the size of the update; preemptively at that since the user has no
> knowledge before tapping download that they may need to remember this number.
> 
> So instead, we can go a more user-centered route and do all the remembering
> and calculations for the user. This is why I'm so intent on telling the user
> how much space they need to clear.
> 
> What would be an even better solution, which was not apparent to me until
> just now, is to skip the error message altogether and right there on that
> screen, image_006, before the user even taps download, tell them they don't
> have enough space to download the update and tell them how much more space
> they need. But I get that time is limited and we've already gone the error
> message route so this idea may have to be a follow-up bug.
> 
> Which in that case, we do need to tell the user how much space is required
> to be freed up. 
> 
> Thanks Shiwei and hopefully that decision makes more sense now that you have
> the rationale and have put yourself in the user's position.

I'm so sorry to reply so late. Yes, you are right. If possible, we should go a more user-centered route, few changes may be significantly better. I'll prepare a new patch as soon as I can.

Comment 89

[Shiwei Zhang]

BTW, warning UI will trigger when free space in /data is below 5MB, if I want adjust this threshold to a larger size，for example 10MB，where can I made it?

Comment 90

[[:SalvadorR]]

This issue also occurs when there is no SD card in the device

Environmental Variables:
Device: Flame 2.1 (319mb) KK FF
BuildID: 20141017001201
Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98
Gecko: 2befa902ff5c
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 91

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm clearing the blocking 1.3t+ flag because I'm sick of seeing this on the release dashboard.

Comment 92

[Francesco Lodolo [:flod]]

Removing late-l10n keyword, since nobody seems to be working on this.

Comment 93

[Tiffanie Shakespeare [:tif]]

I'm guessing this is still an issue? Following the comments it looks like the ball is in  Shiwei's court?

Comment 94

[Shiwei Zhang]

(In reply to Tiffanie Shakespeare from comment #93)
> I'm guessing this is still an issue? Following the comments it looks like
> the ball is in  Shiwei's court?

Sorry for reply so late, but I get plenty to do these days, UI_for_no_space_update_m.patch in my comment80 is nearly completed, and I think it is complete in a sense or making a few changes will be perfect.

Dear Tiffanie, please reconsider it and change it if necessary, I have stacks of work waiting to be done ，it is hard for me to make any update recently.

Thanks a lot.

Comment 95

[Tiffanie Shakespeare [:tif]]

Shiwei - I totally know how that goes, the UX team is super busy with our next release efforts. Unfortunately, I'm not a developer and can't make any code changes to your patch. 

Update when you can or perhaps we can re-assign it (if that's cool with you). Enjoy and good luck with your work!

Comment 96

[Eric Tsai (no more review request accepted)]

Shiwei, I think I can help to improve the patch, like Comment 86. Any more idea to improve?

Comment 97

[Shiwei Zhang]

(In reply to Eric Tsai from comment #96)
Thank you Eric, I have no idea.

We don't have Tarako environment to develop patch right now, 

Maybe we can raise a new bug on latest versions, for instance, v2.1.

Comment 98

[Eric Tsai (no more review request accepted)]

I think we can land the patch to m-c for all devices. Latest version doesn't cover this either.

Comment 99

[Eric Tsai (no more review request accepted)]

I take this bug for 3.0 and improve Comment 86.

Comment 100

[Eric Tsai (no more review request accepted)]

Attachment: No free space dialog when download an update

NI jelee for UI review. Please give comment for this dialog and string.

Comment 101

[Jenny Lee]

ni Matej to review the string, thanks Matej!

Comment 102

[Jenny Lee]

Attachment: dialog_sample.png

Hi Eric,

See attached for preferred dialog styling.
Could you please modify the current screen accordingly? Thanks!

Comment 103

[Matej Novak [:matej]]

(In reply to Eric Tsai from comment #100)
> Created attachment 8602037 [details]
> No free space dialog when download an update
> 
> NI jelee for UI review. Please give comment for this dialog and string.

I would make it:

"You only have 29.08MB of free space. Please delete some files before trying your update again."


Another way to do it is to not list the amount of free space, which may not mean much to a user anyway:

"You don't have enough free space. Please delete some files before trying your update again."


Or let them know how much space is required:

"You need XXMB of free space. Please delete some files before trying your update again."

Comment 104

[Tiffanie Shakespeare [:tif]]

I agree with Matej's last suggestion and something similar was actually my recommendation in comment #66 - great minds think alike :) 

By letting the user know how much space is required, the user can then decide what to delete, how much, and whether it's worth the effort.

Comment 105

[Stephany Wilkes]

Tif submitted suggestions for this nearly one year ago. Please land this and let's end the string discussion ASAP.

Comment 106

[Stephany Wilkes]

Attachment: 2015-04-23 15.26.07.jpg

When Tif submitted the string, this baby was not born. Now he is old enough to play in a box. This is how long this bug has been in progress. Let's finish this! :-)

Comment 107

[Eric Tsai (no more review request accepted)]

Attachment: No free space dialog when download an update, with update's size

Update dialog style and modified string

Comment 108

[Matej Novak [:matej]]

(In reply to Eric Tsai from comment #107)
> Created attachment 8603223 [details]
> No free space dialog when download an update, with update's size
> 
> Update dialog style and modified string

Is the number how much extra space is required or the total space required for the update? I was imagining it would be the latter so it never has to change.

Seeing it in layout, I would tweak as follows:

"This update requires XX MB of free space. Please delete some files and try again."

Comment 109

[Tiffanie Shakespeare [:tif]]

Matej - It would be more user friendly to inform the user how much space they need to free up. That way the user doesn't have to get into the situation where they delete a couple photos run into this error again, delete a few more, etc.

If you want would could include both pieces of information: "This update requires XX MB of free space. You need to free up xx MB. Please delete some files and try again."

Or something to that affect. Essentially, we are doing the math for the user (update - device free space). See my comment 85 for the more about the rationale of including how much spac. Thanks!

Comment 110

[Matej Novak [:matej]]

Could it be as simple as this?

"This update requires XX MB of free space. Please delete at least XX MB and try again."

Comment 111

[Gregor Wagner [:gwagner]]

The OTA file is downloaded and extracted in the system partition. The user doesn't have access to this partition. Deleting downloads, videos or other files won't help since they are stored in another partition.

Comment 112

[Tiffanie Shakespeare [:tif]]

Gregor - how does the user recover from the error then?

Comment 113

[Eric Tsai (no more review request accepted)]

(In reply to Matej Novak [:matej] from comment #108)
> Is the number how much extra space is required or the total space required
> for the update? I was imagining it would be the latter so it never has to
> change.

Yes, my screenshot is latter.

(In reply to Gregor Wagner [:gwagner] from comment #111)
> The OTA file is downloaded and extracted in the system partition. The user
> doesn't have access to this partition. Deleting downloads, videos or other
> files won't help since they are stored in another partition.

:gwagner and Tiffanie, I think it is also used for FOTA so from previous patch, it will detect how much free space in sdcard. For FOTA, user can easily clean sdcard. But :gwagner is right, if it's OTA, we might not able to recovery this.

Comment 114

[Jenny Lee]

(In reply to Eric Tsai from comment #107)
> Created attachment 8603223 [details]
> No free space dialog when download an update, with update's size
> 
> Update dialog style and modified string

Hi Eric,

Looks good except that the title isn't left aligned. Thanks!

Comment 115

[Gregor Wagner [:gwagner]]

(In reply to Tiffanie Shakespeare from comment #112)
> Gregor - how does the user recover from the error then?

There is not much the user can do in an OTA case :/

Comment 116

[Stephany Wilkes]

So if I understand this correctly... it took us one year to realize this isn't actually a bug?

Comment 117

[Eric Tsai (no more review request accepted)]

This could happen on FOTA and OTA. User can do something on FOTA but nothing for OTA https://bugzilla.mozilla.org/show_bug.cgi?id=1004749#c115 . But developer can find something wrong for OTA. It's worth to solve this.

Comment 118

[BMO Automation]

Firefox OS is not being worked on