Notify the user about the low free storage situation

RESOLVED FIXED in Firefox 23

Status

Firefox OS
Gaia::System
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: On parental leave, Assigned: On parental leave)

Tracking

(Blocks: 1 bug, {late-l10n})

unspecified
1.0.1 IOT1 (10may)
ARM
Gonk (Firefox OS)
late-l10n
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)

Details

Attachments

(7 attachments, 7 obsolete attachments)

(Assignee)

Description

5 years ago
Once we get notifications from the fanotify based component being build in bug 861898, we need to let the user know about the low storage situation.
(Assignee)

Updated

5 years ago
Blocks: 861898
Depends on: 853350
(Assignee)

Updated

5 years ago
blocking-b2g: --- → tef?

Updated

5 years ago
Component: Gaia → Gaia::System

Comment 1

5 years ago
Bug 853350 already has a proposed UX spec. Should we dupe to that?
Keywords: late-l10n
(Assignee)

Comment 2

5 years ago
I don't think we should dupe to Bug 853350. That bug will track the creation of a device space watcher. I'll move the proposed UX spec to this bug.
Assignee: nobody → ferjmoreno
(Assignee)

Comment 3

5 years ago
Created attachment 737987 [details]
Low Storage UX specs, v2
(Assignee)

Comment 4

5 years ago
Josh, thanks for the UX spec proposal, a few comments about it:

1. It would be great if we could also add a persistent notification about the low storage condition while it exists. Something like a notification bar icon that reminds the user about the current state of her device.

2. The first banner for the "Device Storage Warning" page has the same text as the second one. Is it intentional?

3. Answering to your question about the distinction between app types, we can probably do that, but I don't think we should. The idea is to avoid installing *any* kind of apps. Hosted apps also use device storage (for the app manifest for example). In any case, the notification for not enough device storage while installing apps is already implemented and working.
Flags: needinfo?(jcarpenter)
Triage 4/17 agrees tef+ per 853350
blocking-b2g: tef? → tef+
(Assignee)

Comment 6

5 years ago
Created attachment 738730 [details] [diff] [review]
B2G part WIP
(Assignee)

Comment 7

5 years ago
Created attachment 738731 [details]
Gaia part - PR link

Pointer to Github pull-request
(Assignee)

Updated

5 years ago
Attachment #738731 - Attachment description: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9252 → Gaia part WIP
Can't we do that via the app: part of the device storage API directly in Gaia. Seems like this is what http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/FreeSpaceWatcher.jsm is doing anyway.
Whiteboard: [status: has WIP patches]
We don't want to poll continuously, that's a waste of resource compared to the real-time info we get from the kernel. Also, doing it in gaia would mean sending back the notification from gaia to gecko which looks like the wrong way.

We used the timer + device storage in FreeSpaceWatcher.jsm because the monitoring there is short-lived. We'll convert it to use the new API in a followup.
(Assignee)

Comment 10

5 years ago
Indeed. I've filed Bug 863596
(In reply to Fabrice Desré [:fabrice] from comment #9)
> We don't want to poll continuously, that's a waste of resource compared to
> the real-time info we get from the kernel. Also, doing it in gaia would mean
> sending back the notification from gaia to gecko which looks like the wrong
> way.

I was not suggesting to move the apps code in Gaia but to implement this feature in Gaia directly.

> 
> We used the timer + device storage in FreeSpaceWatcher.jsm because the
> monitoring there is short-lived. We'll convert it to use the new API in a
> followup.

I didn't know about this new API, it sounds better obviously. BUt instead of using our dirty little mozChromeEvent / mozContentEvent, I wonder if that would be better to fire an event on the deviceStorage returned by a call to getDeviceStorage('apps');

The low level code can listen for the observers messages.
Spoke about this with Fernando and Fabrice and we're aligned for 1.0.1 and 1.1 for UX. We'll use a single notification when the available device storage drops below the low storage threshold. Notification copy should be: 

Line 1: "Device space low:" 
Line 2: "X MB remaining"

Where X is the amount remaining. 

Notification is then pinned inside the Notification Tray, to the top, until the condition is cleared. Ideally "X MB remaining" will continue to update, but if that is not possible, let's remove it, since we do not want it to become inaccurate over time.

Clicking the notification should take the user to Settings > Device Storage. Which is admittedly not very helpful in v1.0.1 and 1.1, but is probably the best we can do. We're open to better options, however.
Flags: needinfo?(jcarpenter)
Whiteboard: [status: has WIP patches] → [status: has WIP patches, blocked on kernel change]
(Assignee)

Comment 13

5 years ago
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #11) 
> I didn't know about this new API, it sounds better obviously. BUt instead of
> using our dirty little mozChromeEvent / mozContentEvent, I wonder if that
> would be better to fire an event on the deviceStorage returned by a call to
> getDeviceStorage('apps');
> 
> The low level code can listen for the observers messages.

We can do that.

I don't have an strong opinion about how to expose this event. We could:

A. Use the current "onchange" event [1], adding extra information about the free space and a "lowStorage" flag.

B. Use a new "onlowstorage" event that would include the remaining free space and a flag indicating if the notification is for a low storage situation or because the device has been recovered to a healthy storage situation.

C. Use two events "onlowstorage"/"onfreestorage" containing the free space in both cases.

[1] https://mxr.mozilla.org/mozilla-central/source/dom/interfaces/devicestorage/nsIDOMDeviceStorage.idl#22
Flags: needinfo?(doug.turner)
Does the event really need to show how much space exists in a given mount point?  I am not sure that really means anything to most users.  Josh?



onchange is probably the right thing.  Currently we have the following:

  readonly attribute DOMString path;
  readonly attribute DOMString reason;

Maybe we just add a new reason "low-disk-space" -or- something similar sounding.  We should probably also dispatch another onchange when the space is 'okay' again.

Does this make sense?
Flags: needinfo?(doug.turner)
(In reply to Doug Turner (:dougt) from comment #14)
> Does the event really need to show how much space exists in a given mount
> point?  I am not sure that really means anything to most users.  Josh?

Right now this is the only way a user can know how much space he cleared by uninstalling an application. That's not ideal but we can't do better for v1.
(Assignee)

Comment 16

5 years ago
Thanks Doug!

I guess we can also use nsIDOMDeviceStorage.freeSpace [1] instead of including that information within the event.

[1] https://mxr.mozilla.org/mozilla-central/source/dom/interfaces/devicestorage/nsIDOMDeviceStorage.idl#41
(Assignee)

Comment 17

5 years ago
Created attachment 741422 [details] [diff] [review]
Gecko part - v1

This patch triggers an 'onchange' event with "low-disk-space"/"available-disk-space" as reason and with an empty path.

I'll be writing some tests for it asap.
Attachment #738730 - Attachment is obsolete: true
Attachment #741422 - Flags: review?(dhylands)
(Assignee)

Comment 18

5 years ago
Comment on attachment 738731 [details]
Gaia part - PR link

I've just updated the Gaia PR. Etienne, could you provide some feedback about it, please? It is just a WIP, but it is currently working on my tests.
Attachment #738731 - Flags: feedback?(etienne)
(Assignee)

Updated

5 years ago
Attachment #741422 - Attachment description: v1 → Gecko part - v1
(Assignee)

Comment 19

5 years ago
Josh, do you think that we'd need an icon for the notification or the message is enough?
Flags: needinfo?(jcarpenter)

Comment 20

5 years ago
(In reply to Josh Carpenter [:jcarpenter] from comment #12)
> Line 1: "Device space low:" 
> Line 2: "X MB remaining"
> 
> Where X is the amount remaining. 

We could just say:

Line 2: "< X MB remaining" OR "Less than X MB remaining"

This wouldn't require the remaining storage amount to be updated.  

As for an icon, if we have a 'warning-like' icon already today, maybe we could re-use that?

I'll defer to Josh/Patryk on what is available already.
Comment on attachment 738731 [details]
Gaia part - PR link

I'll do some preemptive nit picking on github but this looks good!
Attachment #738731 - Flags: feedback?(etienne) → feedback+
Comment on attachment 741422 [details] [diff] [review]
Gecko part - v1

Review of attachment 741422 [details] [diff] [review]:
-----------------------------------------------------------------

r=me if you fixup the location that Add/RemoveObserver is called for dsik-space-watcher

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +2449,5 @@
>  
>      DeviceStorageFile* file = static_cast<DeviceStorageFile*>(aSubject);
>      Notify(NS_ConvertUTF16toUTF8(aData).get(), file);
>      return NS_OK;
> +  } else if (!strcmp(aTopic, "disk-space-watcher")) {

Since device storage primarily deals with media storage, and this is fo app storage, i think it would be worthwhile to add a comment that indicates that these messages are primarily for app storage.

@@ +2562,5 @@
>                                                       win, mPrincipal, dsf, request, this, aListener);
>    NS_DispatchToMainThread(r);
> +
> +  nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
> +  obs->AddObserver(this, "disk-space-watcher", false);

This feels like its in the wrong place to be registering this observer. Shouldn't the disk-space-watcher observer be added in nsDOMDeviceStorage::SetRootDirectoryForType and removed in nsDOMDeviceStorage::Shutdown?

At least that's where the other observers are registered.
Attachment #741422 - Flags: review?(dhylands) → review+
(Assignee)

Comment 23

5 years ago
Etienne, Dave, thanks for your feedback!

Etienne, I've just updated the PR with some tests and modifications. I'd like to review it myself before asking for your review though.
(Assignee)

Updated

5 years ago
Attachment #738731 - Flags: review?(etienne)
(Assignee)

Comment 24

5 years ago
Comment on attachment 738731 [details]
Gaia part - PR link

Stas, could you review the l10n changes, please?
Attachment #738731 - Flags: review?(stas)
(Assignee)

Comment 25

5 years ago
Created attachment 743034 [details] [diff] [review]
Gecko part - v2

This patch addresses Dave's feedback from comment 22.

r+ from comment 22.
Attachment #741422 - Attachment is obsolete: true
Attachment #743034 - Flags: review+
Comment on attachment 738731 [details]
Gaia part - PR link

Please move the size units into the localization file.  You can use existing strings (not sure why we need them defined in two places):

https://github.com/mozilla-b2g/gaia/blob/8701166b9864e2f369c022653826cd6da00e2e12/apps/system/locales/system.en-US.properties#L68

or 

https://github.com/mozilla-b2g/gaia/blob/8701166b9864e2f369c022653826cd6da00e2e12/apps/system/locales/system.en-US.properties#L206

("bytes" is problematic as it should have proper plural form;  file a new bug to get it fixed?)


 +free-space={{freeSpace}} remaining 

This should use plurals because some languages might require to accord the adjective with the number, too.  See https://wiki.mozilla.org/L10n:B2G/Developers#Plurals

Lastly, please attach your next patch to this bug as an actual text attachment, not a link to the pull request.  It's easier for me to just use bugzilla's splinter to comment on code and I like the fact that all discussion about code stays in bugzilla.  Thank you.
Attachment #738731 - Flags: review?(stas) → review-
(Assignee)

Comment 27

5 years ago
Thanks for the feedback Stas!

(In reply to Staś Małolepszy :stas (needinfo along with cc, please; if you want an r+ from me, attach your patch to the bug; I don't review pull requests) from comment #26)
> Lastly, please attach your next patch to this bug as an actual text
> attachment, not a link to the pull request.  It's easier for me to just use
> bugzilla's splinter to comment on code and I like the fact that all
> discussion about code stays in bugzilla.  Thank you.

Sorry about that, I wasn't aware of your preference. Pull requests (and its corresponding link attached to the bug) are the format requested for Gaia patches. I'll attach both, a text attachment and a PR link, next time so everyone reviewing this code can take a look at his preferred format.

Updated

5 years ago
Target Milestone: --- → 1.0.1 IOT1 (10may)
(In reply to Chris Lee [:clee] from comment #20)
> (In reply to Josh Carpenter [:jcarpenter] from comment #12)
> > Line 1: "Device space low:" 
> > Line 2: "X MB remaining"
> > 
> > Where X is the amount remaining. 
> 
> We could just say:
> 
> Line 2: "< X MB remaining" OR "Less than X MB remaining"
> 
> This wouldn't require the remaining storage amount to be updated.  
> 
> As for an icon, if we have a 'warning-like' icon already today, maybe we
> could re-use that?
> 
> I'll defer to Josh/Patryk on what is available already.

"Less than X MB remaining" makes sense. Good call. Re: an icon, I'll ping Patryk / Eric separately and see if they have anything.
Flags: needinfo?(jcarpenter)
Created attachment 743609 [details]
Device Storage Icon

Hi, these are the app permission icons we use for device storage so I think they will make sense here as well.  Let me know if they need any adjustments. Thanks!
(Assignee)

Comment 30

5 years ago
Created attachment 743640 [details] [diff] [review]
Gaia part

Stas, I think this patch addresses your suggestions. I've used the strings at [1] for byte units and used plurals for the "free-space" string.

I've also filed Bug 867180.

[1] https://github.com/mozilla-b2g/gaia/blob/8701166b9864e2f369c022653826cd6da00e2e12/apps/system/locales/system.en-US.properties#L206
Attachment #743640 - Flags: review?(stas)
(Assignee)

Comment 31

5 years ago
Comment on attachment 738731 [details]
Gaia part - PR link

Etienne, I've updated the PR with the last changes. Could you take a look at it, please? Thanks!
Attachment #738731 - Attachment description: Gaia part WIP → Gaia part - PR link
Attachment #738731 - Flags: review-
(Assignee)

Comment 32

5 years ago
Created attachment 743644 [details]
Screenshot of the notification

Eric, thanks for the icon. This is how it looks like in the notification area. Let me know if you want me to do any change, please.
Comment on attachment 738731 [details]
Gaia part - PR link

I wanted to nit pick a bit more but couldn't find anything :)
Attachment #738731 - Flags: review?(etienne) → review+
Whiteboard: [status: has WIP patches, blocked on kernel change] → [status: needs l10n review]
Comment on attachment 743640 [details] [diff] [review]
Gaia part

Axel may be back in the office before Stas - moving the l10n review over to him.
Attachment #743640 - Flags: review?(stas) → review?(l10n)

Comment 35

5 years ago
Comment on attachment 743640 [details] [diff] [review]
Gaia part

Review of attachment 743640 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think patch works, and it has a bunch of l12y bugs. I also expect it to regress the crash UI due to changing whitespace there.

::: apps/system/js/storage_watcher.js
@@ +65,5 @@
> +    var notification;
> +    if (typeof space !== 'undefined') {
> +      notification = msg + ':\n' + this._('free-space', {value: space});
> +    } else {
> +      notification = msg + ':\n' + this._('unknown-free-space');

Don't hardcode strings, ":\n" should be part of the localizable content.

@@ +77,5 @@
> +
> +  updateAvailableSpace: function dsw_updateAvailableSpace(space) {
> +    this._availableSpace.innerHTML = (typeof space !== 'undefined') ?
> +                                     this._('free-space', {value: space}) :
> +                                     this._('unknown-free-space');

innerHTML really? That's insecure, and slow, can you get away with textContent?

@@ +132,5 @@
> +        req.onsuccess = function onsuccess() {
> +          var free;
> +          if (typeof req.result !== 'undefined') {
> +            var formatedSize = self.formatSize(req.result);
> +            free = formatedSize.size + ' ' + formatedSize.units;

Please don't hardcode ' '. There's an existing example on how to use units right, you should mirror what fileSize does.

# Localization note (fileSize*): The string is a float.
fileSize = {{size}} {{unit}}

Also, you're passing a string to something, and then use the plural form? How does that even work?

::: apps/system/locales/system.en-US.properties
@@ +185,4 @@
>  crash-reports-description-2=This may include things like open pages and apps, text typed into forms and the content of open messages, recent browsing history, or geolocation used by an open app.
>  # Localization note (crash-reports-description-3-*): These strings are a paragraph, with a "privacy policy"
>  # link in the middle. Include trailing spaces as needed.
> +crash-reports-description-3-start=We use crash reports to try to fix problems and improve our products. We handle your information as we describe in our

The removal of end-of-line whitespace is likely bad for this UI.

@@ +310,5 @@
> +free-space[one]=Less than {{value}} remaining
> +free-space[two]=Less than {{value}} remaining
> +free-space[few]=Less than {{value}} remaining
> +free-space[many]=Less than {{value}} remaining
> +free-space[other]=Less than {{value}} remaining

Once this uses value and unit independently, there should be a comment on what the two are.
Attachment #743640 - Flags: review?(l10n) → review-
(In reply to Fernando Jiménez Moreno [:ferjm] (offline until 3rd of May) (use needinfo instead of CC, please) from comment #32)
> Created attachment 743644 [details]
> Screenshot of the notification
> 
> Eric, thanks for the icon. This is how it looks like in the notification
> area. Let me know if you want me to do any change, please.

Hi Fernando,

Thanks for implementing this!  I misunderstood where the icon was being used (so I made it the wrong size and style).  I'll attach the correct ones and if you can use them as a replacement that would be great.  Can you flag me for info once they are swapped out so I take a look and make sure they look right? Thanks!
Created attachment 744290 [details]
Device Storage Icon

Hi Fernando, here are the re-sized and restyled low storage icons. Thanks!
Attachment #743609 - Attachment is obsolete: true
Flags: needinfo?(ferjmoreno)
(Assignee)

Comment 38

5 years ago
Created attachment 744589 [details] [diff] [review]
Gaia part - v2

Thanks for the feedback Axel. This patch tries to address all your comments. Please, let me know if I missed anything.
Attachment #743640 - Attachment is obsolete: true
Attachment #744589 - Flags: review?(l10n)
Flags: needinfo?(ferjmoreno)
(Assignee)

Comment 39

5 years ago
Created attachment 744591 [details]
Screenshot

Thanks for the new icons Eric! This is how it looks like now.
Attachment #743644 - Attachment is obsolete: true

Comment 40

5 years ago
Comment on attachment 744589 [details] [diff] [review]
Gaia part - v2

Review of attachment 744589 [details] [diff] [review]:
-----------------------------------------------------------------

I'm still confused when the strings are used separately and when combined, and what that means.

I found a technical nit below, but this should be OK string wise.

If possible, avoid even the '+' of the two localized strings. But I really can't follow the code path to see if that is.

You could either turn the first string into taking the free-space as argument, or create a third combinator, {{message}}\n{{free_space}} or so.

Setting this as feedback+, I'm not really a reviewer at this point.

::: apps/system/js/storage_watcher.js
@@ +102,5 @@
> +    var i = 0;
> +    while (size >= 1024) {
> +      size /= 1024;
> +      ++i;
> +    }

you should terminate this loop so that i remains inside units. Yes, silly to expect more than 1000 YB free space, but still.
Attachment #744589 - Flags: review?(l10n) → feedback+
(In reply to Fernando Jiménez Moreno [:ferjm] (offline until 3rd of May) (use needinfo instead of CC, please) from comment #39)
> Created attachment 744591 [details]
> Screenshot
> 
> Thanks for the new icons Eric! This is how it looks like now.

No Problem, thanks for updating!  But the icon looks smaller then intended, can you check if it is being scaled down from 30x30px? Thanks!
Flags: needinfo?(ferjmoreno)

Comment 42

5 years ago
UX team, is the text "Device space low" the right text?
Flags: needinfo?(firefoxos-ux-bugzilla)
(In reply to Chris Lee [:clee] from comment #42)
> UX team, is the text "Device space low" the right text?

I believe the text should be "Less than X MB remaining"
Josh can you confirm? Thanks!
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(jcarpenter)

Comment 44

5 years ago
Eric, that's more in line with existing strings (I just checked on the Mercurial string repo) and the copy style guide, so let's go with that. Clearing Josh so we can keep this moving. Thanks, Eric!
Flags: needinfo?(jcarpenter)
(Assignee)

Comment 45

5 years ago
As you can see in comment 12, Josh already defined the text to be shown as:

Line 1: "Device space low" 
Line 2: "X MB remaining"

After that, in comment 28, he agreed to change line 2 to:

"Less than X MB remaining"

And that is what it is currently implemented and what you can see in the attached screenshot.



Line 1: "Device space low."
Line 2: "Less than 194 MB remaining."(In reply to Eric Pang [:epang] from comment #41)

> (In reply to Fernando Jiménez Moreno [:ferjm] (offline until 3rd of May)
> (use needinfo instead of CC, please) from comment #39)
> > Created attachment 744591 [details]
> > Screenshot
> > 
> > Thanks for the new icons Eric! This is how it looks like now.
> 
> No Problem, thanks for updating!  But the icon looks smaller then intended,
> can you check if it is being scaled down from 30x30px? Thanks!

You are right! It was being scaled. Sorry about that. I am updating the PR and the screenshot.
Flags: needinfo?(ferjmoreno)
(Assignee)

Comment 46

5 years ago
Created attachment 745032 [details]
Screenshot
Attachment #744591 - Attachment is obsolete: true
(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #46)
> Created attachment 745032 [details]
> Screenshot

Thanks, sizing looks right now, but is it possible to shift the icon to the left by 3 pixels?  Sorry for all the adjustments, this will allow it to be centered under the download icon.  Let me know if it's doable, Thanks!
Flags: needinfo?(ferjmoreno)

Updated

5 years ago
Whiteboard: [status: needs l10n review]
(Assignee)

Comment 48

5 years ago
Created attachment 746407 [details]
Screenshot

Eric, I've just updated the PR and screenshot. Let me know if this looks correct now, please. Thanks!
Attachment #745032 - Attachment is obsolete: true
Flags: needinfo?(ferjmoreno)
(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #48)
> Created attachment 746407 [details]
> Screenshot
> 
> Eric, I've just updated the PR and screenshot. Let me know if this looks
> correct now, please. Thanks!

This looks good now Fernando.  Thanks for all the updates!
Whiteboard: [status: r+'d patches, needs bug 853350]
The gecko patch here does not apply to current m-c. Fernando, can you update it and land this now that bug 853350 is landed on birch?
(Assignee)

Comment 51

5 years ago
http://hg.mozilla.org/projects/birch/rev/1d1c4c086287
(Assignee)

Comment 52

5 years ago
https://github.com/mozilla-b2g/gaia/commit/fad3da3881351a6a2b66fcddb2fe2dd0b72eddb2

Updated

5 years ago
Blocks: 870735
(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #51)
> http://hg.mozilla.org/projects/birch/rev/1d1c4c086287

Let's not wait on full resolution to uplift given the deadline here - Ryan/John, can you uplift to v1.1/v1.0.1 please?
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: needinfo?(ryanvm)
Flags: needinfo?(jhford)
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [status: r+'d patches, needs bug 853350] → [status: needs uplift]

Updated

5 years ago
Keywords: verifyme
QA Contact: jsmith
Please don't mess with bug resolutions. It doesn't help.
Status: RESOLVED → REOPENED
Flags: needinfo?(ryanvm)
Resolution: FIXED → ---
(Assignee)

Comment 55

5 years ago
Created attachment 748056 [details] [diff] [review]
Gecko part - b2g18

This patch applies in b2g18
Attachment #748056 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/1d1c4c086287
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ddcb5f19016a
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/a0b3e974355a

Leaving the status flags set to affected for the Gaia uplift.
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → affected
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed
Whiteboard: [status: needs uplift]
Commit fad3da3 does not apply cleanly to v1-train or v1.0.1, but both seem to have the same conflict.  This looks like a fairly simple resolution, but I'd prefer that someone more familiar with the code look at it.

To resolve this, please:

  cd gaia
  git checkout v1-train
  git cherry-pick -x -m1 fad3da3
  <resolve merge conflict>
  git add apps/system/index.html apps/system/locales/system.en-US.properties
  git commit
  git checkout v1.0.1
  git cherry-pick $(git rev-parse v1-train)
Flags: needinfo?(jhford)
(Assignee)

Comment 59

5 years ago
v1.0.1: https://github.com/mozilla-b2g/gaia/commit/0d46a210900d7a27168de4cec6cafdd52d9d85bc
v1-train: https://github.com/mozilla-b2g/gaia/commit/ebe14f72aab0a306c33e4191a841e1c3c79e79af
status-b2g18: affected → fixed
status-b2g18-v1.0.1: affected → fixed
Looks like the cherry-picks in comment 59 aren't quite right.  These two lines added to apps/system/index.html were not part of the original patch to gaia/master,

+    <!-- Shared responsive -->
+    <link rel="stylesheet" href="shared/style/responsive.css">

and cause bustage:

"build/webapp-zip.js:311: Error: Can't add inexistent file to zip : .../gaia/shared/style/responsive.css from: system.gaiamobile.org"
Flags: needinfo?(ferjmoreno)
(In reply to Michael Vines [:m1] [:evilmachines] from comment #60)
> Looks like the cherry-picks in comment 59 aren't quite right.  These two
> lines added to apps/system/index.html were not part of the original patch to
> gaia/master,
> 
> +    <!-- Shared responsive -->
> +    <link rel="stylesheet" href="shared/style/responsive.css">
> 
> and cause bustage:
> 
> "build/webapp-zip.js:311: Error: Can't add inexistent file to zip :
> .../gaia/shared/style/responsive.css from: system.gaiamobile.org"

This is filed in bug 871210
(Assignee)

Comment 62

5 years ago
Fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=871210#c1

Sorry for the inconvenience.
Flags: needinfo?(ferjmoreno)
Local storage & appcache tests repeatedly have shown this to work - both warning the user of the low storage when it happens along with having the persistent notification while you remain in low storage.
status-b2g18-v1.0.1: fixed → verified
Keywords: verifyme
Blocks: 1018346

Updated

3 years ago
Blocks: 1071530

Updated

3 years ago
Blocks: 1091374
Depends on: 1105648
You need to log in before you can comment on or make changes to this bug.