Closed Bug 1004749 Opened 8 years ago Closed 4 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v1.3T affected, b2g-v1.4 affected, b2g-v2.0 affected, b2g-v2.1 affected, b2g-v2.2 affected, b2g-master affected)

RESOLVED WONTFIX
Tracking Status
b2g-v1.3T --- affected
b2g-v1.4 --- affected
b2g-v2.0 --- affected
b2g-v2.1 --- affected
b2g-v2.2 --- affected
b2g-master --- affected

People

(Reporter: nhirata, Assigned: etsai)

Details

(Whiteboard: interaction-design, [2.0-flame-test-run-2] [2.1-flame-test-run-3])

Attachments

(10 files, 9 obsolete files)

254.62 KB, text/plain
Details
30.30 KB, image/png
Details
9.66 KB, text/plain
Details
1.01 MB, image/jpeg
Details
2.24 KB, patch
Details | Diff | Splinter Review
5.95 KB, patch
timdream
: review+
Details | Diff | Splinter Review
1.25 MB, image/jpeg
Details
186.36 KB, image/png
Details
1.58 MB, image/jpeg
Details
33.79 KB, image/png
Details
Attached file 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)
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...
Keywords: qawanted
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?
Flags: needinfo?(fabrice)
Keywords: qawanted
Oh I wonder if we turned that off for memory reasons?
Summary: No UI message to the end user that there's no space left if the data/local and sdcard are filled → [OTA][Tarako] No UI message to the end user that there's no space left if the data/local and sdcard are filled
(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.
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.
Flags: needinfo?(fabrice)
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.
Keywords: qawanted
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.
(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.
Keywords: qawanted
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.
Summary: [OTA][Tarako] No UI message to the end user that there's no space left if the data/local and sdcard are filled → [OTA][Tarako] No Update UI message to the end user that there's no space left if the data/local and sdcard are filled
Right, but that's not tarako specific?
Fabrice, I don't think so.
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 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.
(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?
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
(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.
To clarify the last comment : if the file size is > the space left and the file size is > the notification limit.
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.
Keywords: qawanted
James, just a fyi that we don't have a UI that supports saying how much space we have left on the device.
Flags: needinfo?(jcheng)
Flags: needinfo?(james.zhang)
(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.
Flags: needinfo?(james.zhang)
noming based on comment 20.
blocking-b2g: --- → 1.3T?
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.
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.
Flags: needinfo?(james.zhang)
Loop Arvin, can you provide patch?
Assignee: nobody → arvin.zhang
Flags: needinfo?(james.zhang)
ni? Steven to understand partner's update flow
Flags: needinfo?(jcheng) → needinfo?(styang)
One of the OEM will still host FOTA server, it's good to have this. thanks.
Flags: needinfo?(styang)
blocking-b2g: 1.3T? → 1.3T+
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
(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?
blocking-b2g: 1.3T+ → 1.3T?
(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.
Fair enough.
blocking-b2g: 1.3T? → 1.3T+
Arvin, any update here?
Flags: needinfo?(arvin.zhang)
Hi Gregor,
  
  Sorry to reply late and please shiwei help to update state here.
  Thanks.
Flags: needinfo?(arvin.zhang) → needinfo?(shiwei.zhang)
Assignee: arvin.zhang → shiwei.zhang
Shiwei, what's the status with this bug? Are you working on it?
James, how's the status in your side?
Flags: needinfo?(james.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.
Flags: needinfo?(shiwei.zhang)
Fabrice and Steven, can you help here?
Shiwei, you can ask Jesse for help.
Flags: needinfo?(james.zhang)
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.)
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
(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.
(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?
Attached patch out_of_memory_notification.patch (obsolete) — Splinter Review
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 <schung@mozilla.com>
Date:   Tue Jun 3 17:46:54 2014 +0800

I am very grateful for your kindly support and help.
Attached file 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.
(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.
(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");
--
Last comment here is from 3days ago. What's the update? This is a blocker!
(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.
Kai-Zhen, Thanks again for your kindly help, I think this patch can handle the issue. please gave a review.
(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
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
Attached image 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.
Attached image 2014-06-11-19-31-07.png (obsolete) —
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?
i'd like to have UX review here as well. Ni? Mike
Flags: needinfo?(mtsai)
(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.
You have to officially ask for review on the patch.
Gregor, I think Kai-Zhen's suggestion reasonable. if time is not urgency I will have a try and give a more appropriate patch.
(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.
Attached patch UI_for_out_of_memory.patch (obsolete) — Splinter Review
This patch is more reasonable,could you give a review, thanks.
Attachment #8433810 - Attachment is obsolete: true
Attachment #8437512 - Attachment is obsolete: true
Attachment #8438374 - Attachment is obsolete: true
Attached patch UI_for_out_of_memory.patch (obsolete) — Splinter Review
This patch is more reasonable,could you give a review, thanks. + review ?
Attachment #8439192 - Attachment is obsolete: true
Attachment #8439675 - Flags: review?(timdream)
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".
Attachment #8439675 - Flags: review?(timdream) → ui-review?(firefoxos-ux-bugzilla)
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.
Attachment #8439675 - Flags: ui-review?(firefoxos-ux-bugzilla) → ui-review?(tshakespeare)
Flags: needinfo?(mtsai)
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!!
Attachment #8439675 - Flags: ui-review?(tshakespeare) → ui-review-
Flags: needinfo?(fdjabri)
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?
Flags: needinfo?(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?
Flags: needinfo?(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.
Attached patch UI_for_updates_no_space.patch (obsolete) — Splinter Review
Maybe this patch could fit all above requirements,could you give a review, thanks.
Attachment #8439675 - Attachment is obsolete: true
Attachment #8443353 - Flags: review?(timdream)
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!
Flags: needinfo?(fdjabri)
Whiteboard: interaction-design
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
Attachment #8443353 - Flags: review?(timdream) → review+
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: interaction-design → interaction-design, [2.0-flame-test-run-2]
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Attached patch UI_for_updates_no_space_m.patch (obsolete) — Splinter Review
(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.
Attachment #8443353 - Attachment is obsolete: true
Attachment #8447977 - Flags: review?(timdream)
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.
Attachment #8447977 - Flags: review?(timdream) → review+
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.
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
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 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.
Attachment #8447977 - Flags: ui-review?(tshakespeare)
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
Attachment #8447977 - Flags: ui-review?(tshakespeare) → ui-review-
Any updates here?
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.
Flags: needinfo?(timdream)
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?
Attachment #8447977 - Flags: review+
Flags: needinfo?(timdream) → needinfo?(shiwei.zhang)
Attached patch UI_for_no_space_update.patch (obsolete) — Splinter Review
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.
Attachment #8447977 - Attachment is obsolete: true
Attachment #8453574 - Flags: review?(timdream)
Flags: needinfo?(shiwei.zhang)
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() )
Attachment #8453574 - Flags: review?(timdream) → review+
(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.
Attachment #8453574 - Attachment is obsolete: true
Attachment #8453598 - Flags: review?(timdream)
Attachment #8453598 - Flags: review?(timdream) → review+
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.
Flags: needinfo?(shiwei.zhang)
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!
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.
Keywords: late-l10n
Attached image 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.
Flags: needinfo?(shiwei.zhang)
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.
(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.
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.
(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.
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?
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
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(dharris)
Whiteboard: interaction-design, [2.0-flame-test-run-2] → interaction-design, [2.0-flame-test-run-2] [2.1-flame-test-run-3]
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(dharris)
I'm clearing the blocking 1.3t+ flag because I'm sick of seeing this on the release dashboard.
blocking-b2g: 1.3T+ → ---
Removing late-l10n keyword, since nobody seems to be working on this.
Keywords: late-l10n
I'm guessing this is still an issue? Following the comments it looks like the ball is in  Shiwei's court?
Flags: needinfo?(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.
Flags: needinfo?(shiwei.zhang)
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!
Shiwei, I think I can help to improve the patch, like Comment 86. Any more idea to improve?
Flags: needinfo?(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.
Flags: needinfo?(shiwei.zhang)
I think we can land the patch to m-c for all devices. Latest version doesn't cover this either.
I take this bug for 3.0 and improve Comment 86.
Assignee: shiwei.zhang → etsai
Summary: [OTA][Tarako] No Update UI message to the end user that there's no space left if the data/local and sdcard are filled → [OTA] No Update UI message to the end user that there's no space left if the data/local and sdcard are filled
NI jelee for UI review. Please give comment for this dialog and string.
Flags: needinfo?(jelee)
ni Matej to review the string, thanks Matej!
Flags: needinfo?(jelee) → needinfo?(matej)
Attached image dialog_sample.png
Hi Eric,

See attached for preferred dialog styling.
Could you please modify the current screen accordingly? Thanks!
(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."
Flags: needinfo?(matej)
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.
Tif submitted suggestions for this nearly one year ago. Please land this and let's end the string discussion ASAP.
Attached image 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! :-)
Update dialog style and modified string
Attachment #8602037 - Attachment is obsolete: true
Flags: needinfo?(matej)
Flags: needinfo?(jelee)
(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."
Flags: needinfo?(matej)
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!
Could it be as simple as this?

"This update requires XX MB of free space. Please delete at least XX MB and try again."
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.
Gregor - how does the user recover from the error then?
Flags: needinfo?(anygregor)
(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.
(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!
Flags: needinfo?(jelee)
(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 :/
Flags: needinfo?(anygregor)
So if I understand this correctly... it took us one year to realize this isn't actually a bug?
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.
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.