STR： 1、Insert a full SD card to the phone 2、Open the Contacts app. 3、Click on the Settings icon. 4、Click on the "Export Contacts" button. 5、Click on the "Memory card“. 6、Choose a contact. 7、Click on the "Export" button in the upper right corner of the page. OBSERVED: 8. The "Export to SD Card" overlay never closes. EXPECTED: 9. POP up the Export error "There's not enough space on the memory card to export the contacts", and "Export to SD Card" overlay closed at the same time.

a) When export a contact to memory card, it shoud creates a vcard by the contact firstly. b) Then get the left space size of memory card. c) If left space size is less than the vcard's size, the function onStorage is finally called. The size of left space of memory card is the error parameter of onStorage(error, storage, finalName). d) In onStorage(): if (error) { var reason = error; // numeric error means not enough space available if (parseInt(error, 10) > 0) { reason = 'noSpace'; } finishCallback({ 'reason': reason }, count, error.message); return; } e) However, if the size of left space is zero, the code above cannot be executed，the 'Export error' cannot be displayed.

The Attachment is my solution to the issue. Please help me check whether it is reasonable. Thank you!

Peipei, can you verify this issue on Flame v1.4?

Flame v1.4 has the same problem. When SD card is full, there is no error message pop up and "Export to SD Card" overlay never closes. But when exporting to a full SIM, there is error msg "SIM card is full" pops up.

Comment on attachment 8441341 [details] [diff] [review] Exporterror_cannot_popup_while_SD_FULL.patch Jan can you include a unit test for this fix? Thanks!

btw, forgot to comment that patch LGTM. Thanks.

ni the author of the patch.

QA Wanted to check 1.3 Flame.

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #7) > ni the author of the patch. I'm sorry! I cannot understand what you mean!

(In reply to Jason Smith [:jsmith] from comment #8) > QA Wanted to check 1.3 Flame. This DOES occur on the 1.3 flame base. Device: Flame 1.3 (Base) BuildID: 20140616171114 Gaia: e1b7152715072d27e0880cdc6b637f82fa42bf4e Gecko: e181a36ebafaa24e5390db9f597313406edfc794 Version: 28.0 (1.3) Firmware Version: v122

Sorry, I am a beginner of Firefox os. So I am afraid I cannot write the unit test of the fix. Please help me write it or tell me how to write it. Thank you.

Sergi, can you help Kun with this?

Kun, no worries, will give you a hand here! Cheers, and thanks for the patch!

Comment on attachment 8441341 [details] [diff] [review] Exporterror_cannot_popup_while_SD_FULL.patch Review of attachment 8441341 [details] [diff] [review]: ----------------------------------------------------------------- Redirecting to a contact peer.

Comment on attachment 8446533 [details] PR to 1.4 branch based on previous attachment with unit tests thanks Francisco, I left a tiny comment on GH about the consistency of the variable 'error'

I cannot land this since I'm having a permanent red on branch 1.4 caused by Homescreen tests: 1) [system] system/HomescreenWindow homescreen window instance. homescreen is crashed Homescreen is crashed at foreground:rerender right away.: TypeError: this.app.element is null at atc_do_opening (http://system.gaiamobile.org:8080/js/app_transition_controller.js:156:7) at atc_changeTransitionState (http://system.gaiamobile.org:8080/js/app_transition_controller.js:94:7) at requireOpen (http://system.gaiamobile.org:8080/js/app_transition_controller.js:255:5) at aw_open (http://system.gaiamobile.org:8080/js/app_window.js:1405:7) at (anonymous) (http://system.gaiamobile.org:8080/js/homescreen_window.js:142:9) at callTimer (http://system.gaiamobile.org:8080/common/vendor/sinon/sinon.js:3171:21) at tick (http://system.gaiamobile.org:8080/common/vendor/sinon/sinon.js:3116:23) at (anonymous) (http://system.gaiamobile.org:8080/test/unit/homescreen_window_test.js:118:9) at wrapper (http://system.gaiamobile.org:8080/common/test/mocha_generators.js:62:19) at run (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3709:7) at runTest (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4087:5) at (anonymous) (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4133:7) at next (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4013:1) at (anonymous) (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4022:7) at next (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3970:1) at (anonymous) (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3985:7) at done (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3700:5) at (anonymous) (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3712:9) at (anonymous) (http://system.gaiamobile.org:8080/common/test/mocha_generators.js:46:20) at wrapper (http://system.gaiamobile.org:8080/common/test/mocha_generators.js:73:15) at run (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3709:7) at next (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3979:5) at (anonymous) (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3990:5) at (anonymous) (http://system.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4938:28)

Just push again adding an extra commit to disable the test that is affecting us.

We opened bug 1033408 to check the status of 1.4 branch, we saw that is perma red and we cannot land, or we shouldn't land in that branch. Reading that bug Tim suggest to do an uplift of bug 864178, that is still in process. So marking this that as a blocker of this one, to see if that bug cleans v1.4 branch.

Francisco, if the error on the branch looks completely unrelated to your patch, and you have the same error on the branch than on your PR, I think it's safe to land. Let's be pragmatic :)

(In reply to Julien Wajsberg [:julienw] from comment #20) > Francisco, if the error on the branch looks completely unrelated to your > patch, and you have the same error on the branch than on your PR, I think > it's safe to land. > > Let's be pragmatic :) :) To be honest, really would like to be pragmatic here, but we should be pretty strict on landing in branches that are not master, if I continue landing (don't discard doing that), how I will ask people to merge things with clean travis/gaia-try ... the branch will have a lot of code for unit tests that we are ignoring. Just discovered that after updating with the bug 864178 landed, we continue having a permanent error in v1.4, in this case: https://travis-ci.org/arcturus/gaia/builds/29050912 Is not just an intermitent, which I can leave with them, but same error repeated. So we should be more strict in branches < 2.1 that we are with our master branch.

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #21) > (In reply to Julien Wajsberg [:julienw] from comment #20) > > Francisco, if the error on the branch looks completely unrelated to your > > patch, and you have the same error on the branch than on your PR, I think > > it's safe to land. > > > > Let's be pragmatic :) > > :) > > To be honest, really would like to be pragmatic here, but we should be > pretty strict on landing in branches that are not master, if I continue > landing (don't discard doing that), how I will ask people to merge things > with clean travis/gaia-try ... the branch will have a lot of code for unit > tests that we are ignoring. > > Just discovered that after updating with the bug 864178 landed, we continue > having a permanent error in v1.4, in this case: > > https://travis-ci.org/arcturus/gaia/builds/29050912 Tim fixed this issue and I merged it with the other patches. You should try to rebase :) > > So we should be more strict in branches < 2.1 that we are with our master > branch. I agree with this. When we'll be on TBPL, sheriffs should take a lot more care than we do. In the mean time, I think that as long as we check what the errors are (which is a lot more work than just checking green or red, yep), it's fine to merge.

(In reply to Julien Wajsberg [:julienw] from comment #22) > Tim fixed this issue and I merged it with the other patches. You should try > to rebase :) > Just did and got the gree \o/ :) Thanks a lot for the hard work here folks!

Landed in v1.4 https://github.com/arcturus/gaia/commit/3a177ac41a0230bbdc60de3b946c0cd022852ca5 Thanks!

Found Test Case: https://moztrap.mozilla.org/manage/case/9066/ STR needs to edited to verify this bug to the existing test case.

New test case created in moztrap: https://moztrap.mozilla.org/manage/case/14235/