976481 - [Sora][camera][gallery][video]no sd card, it would appear black screen at special status.

Status: RESOLVED FIXED

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

Description

[sync-1]

Firefox OS v1.3
 Mozilla build ID: 20140208004002
 
 DEFECT DESCRIPTION:
 no sd card,it would appear black screen at special status
 
  REPRODUCING PROCEDURES:
 1.Launch camera;
 2.It would prompt 'No memory card found',press 'Media storage settings';
 3.Press Home key return to 'No memory card found' notification screen ;
 4.Do step 2&3 three times, then double click the 'Media storage settings'.Appear black screen. ->KO
 5.Recover it by pressing home key.
 
 This bug can be reproduced by double click 'Media storage
 settings' button quickly.
 
 Also reproduced in Gallery/music player/video player.
 
  EXPECTED BEHAVIOUR:
 It would not appear black screen.

Comment 1

[Srinivas Vemula]

Which device your are using?
(In reply to sync-1 from comment #0)
> Firefox OS v1.3
>  Mozilla build ID: 20140208004002
>  
>  DEFECT DESCRIPTION:
>  no sd card,it would appear black screen at special status
>  
>   REPRODUCING PROCEDURES:
>  1.Launch camera;
>  2.It would prompt 'No memory card found',press 'Media storage settings';
>  3.Press Home key return to 'No memory card found' notification screen ;
>  4.Do step 2&3 three times, then double click the 'Media storage
> settings'.Appear black screen. ->KO
>  5.Recover it by pressing home key.
>  
>  This bug can be reproduced by double click 'Media storage
>  settings' button quickly.
>  
>  Also reproduced in Gallery/music player/video player.
>  
>   EXPECTED BEHAVIOUR:
>  It would not appear black screen.

Comment 2

[xiupinglong]

(In reply to Srinivas Vemula from comment #1)
> Which device your are using?

We use Sora device using firefox v1.3.

Comment 3

[Srinivas Vemula]

Please try this patch. 

--- Before/B2G/system/core/rootdir/init.rc	2014-01-15 12:43:13.544724500 +0530
+++ After/B2G/system/core/rootdir/init.rc	2014-01-21 09:00:45.867354600 +0530
@@ -20,7 +20,9 @@
     start ueventd
 
 # create mountpoints
     mkdir /mnt 0775 root system
+	mkdir /mnt/sdcard 0000 system system
     import /init.b2g.rc
 
 on init

Comment 4

[Srinivas Vemula]

along with that patch you have to change in device folder device specific init.rc file. That patch we used for LG device
(In reply to xiupinglong from comment #2)
> (In reply to Srinivas Vemula from comment #1)
> > Which device your are using?
> 
> We use Sora device using firefox v1.3.

Comment 5

[Jason Smith [:jsmith]]

Does this reproduce on a 1.1 Buri device?

Comment 6

[xiupinglong]

(In reply to Jason Smith [:jsmith] from comment #5)
> Does this reproduce on a 1.1 Buri device?

1.1 Buri has not the 'No memory card found',press 'Media storage settings', so we can not see this settings dialog.

Comment 7

[Jason Smith [:jsmith]]

Can someone on the Moz side confirm this?

Comment 8

[xiupinglong]

(In reply to Srinivas Vemula from comment #3)
> Please try this patch. 
> 
> --- Before/B2G/system/core/rootdir/init.rc	2014-01-15 12:43:13.544724500
> +0530
> +++ After/B2G/system/core/rootdir/init.rc	2014-01-21 09:00:45.867354600 +0530
> @@ -20,7 +20,9 @@
>      start ueventd
>  
>  # create mountpoints
>      mkdir /mnt 0775 root system
> +	mkdir /mnt/sdcard 0000 system system
>      import /init.b2g.rc
>  
>  on init

We test with this patch on sora device but it can not resolve this issue.

Comment 9

[Srinivas Vemula]

have you made changes device specific init.rc in device folder ?. I will give complete patch of LG device and  you do changes according to your device and try it. 

(In reply to xiupinglong from comment #8)
> (In reply to Srinivas Vemula from comment #3)
> > Please try this patch. 
> > 
> > --- Before/B2G/system/core/rootdir/init.rc	2014-01-15 12:43:13.544724500
> > +0530
> > +++ After/B2G/system/core/rootdir/init.rc	2014-01-21 09:00:45.867354600 +0530
> > @@ -20,7 +20,9 @@
> >      start ueventd
> >  
> >  # create mountpoints
> >      mkdir /mnt 0775 root system
> > +	mkdir /mnt/sdcard 0000 system system
> >      import /init.b2g.rc
> >  
> >  on init
> 
> We test with this patch on sora device but it can not resolve this issue.

Comment 10

[Srinivas Vemula]

Please try this patch and make changes in device specific init.rc as per your device requirement

****************************************
diff -Nru Before/android/device/lge/galbi/device.mk After/android/device/lge/galbi/device.mk
--- Before/android/device/lge/galbi/device.mk	2014-01-15 12:43:09.932724600 +0530
+++ After/android/device/lge/galbi/device.mk	2014-01-21 08:59:45.661324700 +0530
@@ -215,6 +215,10 @@
     qcom.thermal=thermal-engine \
     ro.sf.lcd_density=480
 
+PRODUCT_COPY_FILES += \
+	device/lge/galbi/volume.cfg:system/etc/volume.cfg
+
 # for Gecko
 PRODUCT_PROPERTY_OVERRIDES += \
        ro.moz.has_home_button=0
diff -Nru Before/android/device/lge/galbi/init.galbi.rc After/android/device/lge/galbi/init.galbi.rc
--- Before/android/device/lge/galbi/init.galbi.rc	2014-01-15 12:43:09.852724600 +0530
+++ After/android/device/lge/galbi/init.galbi.rc	2014-01-21 09:00:06.137349100 +0530
@@ -84,6 +84,8 @@
     symlink /storage/emulated/legacy /mnt/sdcard
     symlink /storage/emulated/legacy /storage/sdcard0
     #symlink /mnt/shell/emulated/0 /storage/emulated/legacy

+    symlink /data /sdcard
 
     # Create filesystems if necessary.
diff -Nru Before/android/device/lge/galbi/volume.cfg After/android/device/lge/galbi/volume.cfg
--- Before/android/device/lge/galbi/volume.cfg	1970-01-01 05:30:00.000000000 +0530
+++ After/android/device/lge/galbi/volume.cfg	2014-01-17 17:14:10.909241600 +0530
@@ -0,0 +1 @@
+create sdcard /data
diff -Nru Before/android/system/core/rootdir/init.rc After/android/system/core/rootdir/init.rc
--- Before/android/system/core/rootdir/init.rc	2014-01-15 12:43:13.544724500 +0530
+++ After/android/system/core/rootdir/init.rc	2014-01-21 09:00:45.867354600 +0530
@@ -20,7 +20,9 @@
     start ueventd
 
 # create mountpoints
     mkdir /mnt 0775 root system
+	mkdir /mnt/sdcard 0000 system system
     import /init.b2g.rc
 
 on init
******************************************

(In reply to xiupinglong from comment #8)
> (In reply to Srinivas Vemula from comment #3)
> > Please try this patch. 
> > 
> > --- Before/B2G/system/core/rootdir/init.rc	2014-01-15 12:43:13.544724500
> > +0530
> > +++ After/B2G/system/core/rootdir/init.rc	2014-01-21 09:00:45.867354600 +0530
> > @@ -20,7 +20,9 @@
> >      start ueventd
> >  
> >  # create mountpoints
> >      mkdir /mnt 0775 root system
> > +	mkdir /mnt/sdcard 0000 system system
> >      import /init.b2g.rc
> >  
> >  on init
> 
> We test with this patch on sora device but it can not resolve this issue.

Comment 11

[xiupinglong]

(In reply to Srinivas Vemula from comment #10)
> Please try this patch and make changes in device specific init.rc as per
> your device requirement
> 

Maybe you misunderstand this issue. The issue here is when we really has not a sdcard in the device, the video/music/gallery/camera apps will show a dialog with button "Media storage settings". when double click 'Media storage settings' button quickly, the device screen turn to black.

Comment 12

[Jason Smith [:jsmith]]

Waiting on QA testing before making a blocking decision.

Comment 13

[KTucker [:KTucker][Inactive 3/4/2016]]

This issue reproduces on the latest buri v 1.3.0 Mozilla RIL 

Environmental Variables
Device: Mozilla v 1.3.0 Mozilla RIL
Build ID: 20140226004002
Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/3c39d1e487d9
Gaia: da50b2b62ed96993cf4df22e3d69c435b28506bd
Platform Version: 28.0
Firmware Version: v1.2-device.cfg

The user will encounter a black screen.

STR:

1. Tap on the camera app with no SD Card inserted.
2. Double tap on "Media storage settings". 
3. Once "Media Storage" settings finishes loading, double tap the "Home" button on the device so the user is taken to the home screen that has the E.me search bar.
4. Repeat steps 1-3.

Actual: The user encounters a black screen.

Comment 6 is correct. This issue does not reproduce on the buri v 1.1.0 because the user cannot go to "Media storage" settings from the camera app.

Comment 14

[David Flanagan [:djf]]

If the bug only manifests when the button is double clicked, I'm guessing that the issue is that we're launching two activities one after another and triggering some kind of bug in the window management code.

The straightforward workaround would be to modify the "media storage settings" button so that it disables itself after it has been clicked and doesn't re-enable for a couple of seconds.

That button only appears on devices that have both an internal and an external storage area. I don't have one of those devices, so I won't be able to test a patch for it.  But I'd be happy to review a patch and give pointers to anyone who wants to take this and fix it.

Comment 15

[Preeti Raghunath(:Preeti)]

Hema

Can we have someone from Camera look at this for the workaround?

Comment 16

[Hema Koka [:hema]]

If this bug is in the window management code, perhaps we should first check if it is a easy fix there before we add a workaround (it looks like we are able to reproduce this on Buri too). 

I think folks in Taipei are out on Friday 28th. 

Gregor, is there someone in your team who can help investigate if it is a bug in the window management code today.

Also NI'ing Alive/Tim in case they are online.

Thanks
Hema

Comment 17

[Jason Smith [:jsmith]]

(In reply to Hema Koka [:hema] from comment #16)
> If this bug is in the window management code, perhaps we should first check
> if it is a easy fix there before we add a workaround (it looks like we are
> able to reproduce this on Buri too). 

The window manager bug might be bug 959056. Tim would know for sure though.

> 
> I think folks in Taipei are out on Friday 28th. 
> 
> Gregor, is there someone in your team who can help investigate if it is a
> bug in the window management code today.

I think when triage discussed this we wanted to get the workaround implemented at a minimum in the media code to prevent a double tap on the button. That would remove the blocking factor on this bug.

Although if this ends up being bug 959056 as the window manager problem, then I'm thinking we might just want to fix the root cause instead of continuously hacking around this problem outside of the system app. We've seen bug 959056 pop up multiple times due to another bug in the product triggering it, so fixing the root cause will stop this problem from happening in the future if another bug so happens to trigger it again.

Comment 18

[Matthew Vaughan]

This issue started reproducing on the 12/14/13 1.3 build.

- Last Working -
Device: Buri v1.3 MOZ RIL
BuildID: 20131213004002
Gaia: 888f9df5515a47d2f5806efee77485e05e1e5416
Gecko: dfae9c83bfbc
Version: 28.0a2
Firmware Version: V1.2-device.cfg

- First Broken -
Device: Buri v1.3 MOZ RIL
BuildID: 20131214004003
Gaia: 37142f72c422120401dbc90e1f5e8f689576bb8e
Gecko: f81e19286279
Version: 28.0a2
Firmware Version: V1.2-device.cfg


This appears to be a gaia issue.

last working gaia/first broken gecko: NO REPRO
Gaia  888f9df5515a47d2f5806efee77485e05e1e5416
Gecko f81e19286279

first broken gaia/last working gecko: REPRO
Gaia  37142f72c422120401dbc90e1f5e8f689576bb8e
Gecko dfae9c83bfbc

Push log: https://github.com/mozilla-b2g/gaia/compare/888f9df5515a47d2f5806efee77485e05e1e5416...37142f72c422120401dbc90e1f5e8f689576bb8e

Comment 19

[Hema Koka [:hema]]

Tim/Gregor - please comment if this is related to window management issue...would rather get the root cause fixed if it doable in this timeframe

worst case punam can add the bandage that was suggested just for 1.3

thanks
hema

Comment 20

[David Flanagan [:djf]]

Even if there is a possible fix in the window manager, we are being really unfriendly by allowing our apps to launch multiple activity requests.  We recently fixed a similar bug in the code that launched camera from gallery and vice versa. So we need to fix this in the media apps anyway.  And I suspect that there isn't a really adequate solution in the window manager anyway.

So I'll take this.

Comment 21

[David Flanagan [:djf]]

On further thought, the bug here is that the Media Storage Settings button should just never appear in any of these apps in the first place.

This button was added when we got phones with both internal and external storage areas. The idea was that if the currently selected storage area became unavailable, the user can go to settings and select the other one. But in practice this is never how it worked.  If either storage area is available, the media apps will use it and won't display the dialog.  If the dialog is displayed, it means that no storage area is available, and going to the settings app won't fix that.

These are like zombie buttons that will not die. I've asked that they be removed.  And it has been removed from the video app in v1.4.  I thought we'd gotten rid of the rest as well.  But I guess not.  Now is my chance to get rid of them all in v1.3 and v1.4.

Also, I should note that I can't reproduce the bug on a hamachi.  When I see the button I click it as fast as I can but I never get a black screen.  I would expect this kind of bug to be timing sensistive, so I guess I'm not surprised that it is hardware dependent.

In any case, if I just get rid of the (useless) buttons, then we won't have a bug here anymore.

Comment 22

[David Flanagan [:djf]]

I can get rid of the useless button for 1.3, but because it is too late for localization changes, we're probably stuck with the "or select an alternate default media location in settings" text, unless there are still old strings in the localization files.

Comment 23

[David Flanagan [:djf]]

Attachment: patch for the 1.3 branch

Punam, since Hema almost volunteered to have you do this patch, I'm asking you to review it.

Reporter: would you try this patch and verify that it fixes the bug for you. (Note that the way it fixes the bug is by removing the buttons, since the should not have been there in the first place.)

Comment 24

[David Flanagan [:djf]]

Attachment: patch to simulate having no sdcard

I was unable to remove the sdcard from my hamachi device, so I wrote this little patch to simulate having no sdcard in each of the media apps.  Punam, you can use this in your testing if it helps, though we'll want to be sure to test on a device that really has no sdcard before landing.

(Also, I suppose we should test on a device that has internal storage and an empty sdcard slot to verify that the media apps just work in that situation. I don't of any device other than the Leo like that.)

Comment 25

[David Flanagan [:djf]]

Most of this patch will not uplift cleanly to master, and it might just be easiest to redo it from scratch for 1.4.  But I'll wait to make sure it passes review and is an acceptable fix for the bug.

Comment 26

[xiupinglong]

(In reply to David Flanagan [:djf] from comment #23)
> Created attachment 8383375 [details] [review]
> patch for the 1.3 branch
> 
> Punam, since Hema almost volunteered to have you do this patch, I'm asking
> you to review it.
> 
> Reporter: would you try this patch and verify that it fixes the bug for you.
> (Note that the way it fixes the bug is by removing the buttons, since the
> should not have been there in the first place.)

The patch remove the button "Media storage settings", it can not trigger settings app activity from these media apps. But the issue still exists, when the other app trigger an activity using double click a button, the page will be black too. This is not a root patch IMO.

Comment 27

[David Flanagan [:djf]]

(In reply to xiupinglong from comment #26)

> The patch remove the button "Media storage settings", it can not trigger
> settings app activity from these media apps. But the issue still exists,
> when the other app trigger an activity using double click a button, the page
> will be black too. 

If you can find any examples of that, please file a bug report.  I have only seen issues with "window disposition" activities and most activities we use are "inline disposition" so I don't actually think this will be a significant problem.

> This is not a root patch IMO.

You're unlikely to get anything better for a 1.3+ bug at this stage of the release.

Comment 28

[Guoqiang.CHEN]

Dear David,

I found a strange phenomenon， I forbid continuous response in Activities.handleEvent(activities.js).
Now when I quickly click button for serveral times, it only process activity-choice one times in Activities.handleEvent. But when I press home_key back to this UI, I only click button one time, but in SystemMessageInternal.sendMessage(SystemMessageInternal.js) will send several openAppPage messages, it seems last time serveral clicks caused these messages, but I do not find where systemMessageInternal.sendMessage is called.

Comment 29

[Punam Dahiya [:pdahiya]]

I have tested the patch on v1.3, device hamachi by removing sdcard. The 'media storage settings' button is removed from overlay in camera, music, video and gallery app.

The overlay successfully works for above apps on inserting the sdcard for scenarios such as memory card in use.

The patch looks good and has r+ from me.

Comment 30

[David Flanagan [:djf]]

(In reply to Guoqiang.CHEN from comment #28)
> Dear David,
> 
> I found a strange phenomenon， I forbid continuous response in
> Activities.handleEvent(activities.js).
> Now when I quickly click button for serveral times, it only process
> activity-choice one times in Activities.handleEvent. But when I press
> home_key back to this UI, I only click button one time, but in
> SystemMessageInternal.sendMessage(SystemMessageInternal.js) will send
> several openAppPage messages, it seems last time serveral clicks caused
> these messages, but I do not find where systemMessageInternal.sendMessage is
> called.

Guoqiang,

I am not familiar with any of the code you reference, so I can't help with your question. This bug was filed as a media app bug, and I'm going to resolve it as a media app bug by removing the useless buttons that should not have been there anyway.

If you'd like to pursue a more general fix in the System app, please file a new bug and set needinfo for [e-mail]

Comment 31

[Punam Dahiya [:pdahiya]]

(In reply to xiupinglong from comment #26)
> (In reply to David Flanagan [:djf] from comment #23)
> > Created attachment 8383375 [details] [review]
> > patch for the 1.3 branch
> > 
> > Punam, since Hema almost volunteered to have you do this patch, I'm asking
> > you to review it.
> > 
> > Reporter: would you try this patch and verify that it fixes the bug for you.
> > (Note that the way it fixes the bug is by removing the buttons, since the
> > should not have been there in the first place.)
> 
> The patch remove the button "Media storage settings", it can not trigger
> settings app activity from these media apps. But the issue still exists,
> when the other app trigger an activity using double click a button, the page
> will be black too. This is not a root patch IMO.

Attached fix has addressed the issue as reported in media apps by taking out buttons.

Below are the other use cases where configure activity is invoked from different apps, though none of the below cases has redirect to mediaStorage section.

Please report a bug if you see black screen in any of these cases.

Battery - showPowerSavingNotification 
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/battery_manager.js#L256

SMS -> settings
https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/activity_picker.js#L111

Keyboard ->Built-inKeyboards ->Back - Bug 959056
https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/settings/settings.js#L11

Quick Settings -> Auto-config wifi
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/quick_settings.js#L283

Storage Watcher -> Redirects to app permissions section of the settings app so they can uninstall 3rd party apps on low storage
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/storage_watcher.js#L39

Comment 32

[David Flanagan [:djf]]

Attachment: patch for master

Here's a version of the patch for the master branch. Nothing is cherry-picked here, this is just a separate patch.

Punam: could you review and test this one too?

Justin: the camera code is the stuff I'm least familar with, so I've asked for your review of just that part as well.

Dominic: this patch modifies the music app to remove the "storage settings" button in the overlay and add a cancel button if we're handling a pick activity.  Setting feedback to bring it to your attention.

Note that this patch for master actually changes strings. So instead of just removing the useless "media storage settings" button, it also modifies the "nocard" overlay text so it doens't reference the settings app at all.

Also, in 1.4, Music was the only app that did not display a Cancel button in the overlay when invoked for a pick activity. So for that app, I've converted the settings button to a cancel button.

All four of these apps should do the right thing when there is no sdcard and they are invoked normally and when they are invoked by pick activity (e.g. when invoked from the sms app to add an attachment)

Comment 33

[David Flanagan [:djf]]

Attachment: A patch for master to simulate having no sdcard

I don't currently have a working device that I can remove the sdcard from, so this patch modifies the apps to behave as if there is no sdcard. Attaching it here in case reviewers need it.

Comment 34

[David Flanagan [:djf]]

Landed to v1.3: https://github.com/mozilla-b2g/gaia/commit/9224682ee890946abcb389193a622af6c3ec3d80

But I forgot to ask Fabrice's approval, so I may need to back this out.

Note that this v1.3 patch is imperfect.  It removes the useless buttons but because the localization deadline is past the patch leaves in the misleading overlay text that makes users think they can cure their missing sdcard by visiting the settings app.

Comment 35

[David Flanagan [:djf]]

Leaving the bug open until the patch for master is reviewed and landed.

Comment 36

[(inactive after 6/18) Alive Kuo [:alive]]

Clear needinfo.

Please file a general bug for system app if necessary. Thanks.

I wonder this issue is fixed by bug 976846.

Comment 37

[Punam Dahiya [:pdahiya]]

Comment on attachment 8383913 [details] [review]
patch for master

Hi David

Thanks for 1.4 patch,tested the patch and it looks good. Only issue i see is a unit test failure inside camera 

[camera] controllers/overlay OverlayController#getOverlayData() Should return correct data for 'unavailable':
at (anonymous) (http://camera.gaiamobile.org:8080/test/unit/controllers/overlay_test.js:142:7)


With that fixed, it looks good and has r+. Thanks

Comment 38

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8383913 [details] [review]
patch for master

I agree with Punam, this looks good as long as the unit test gets fixed. See my comment in GitHub as to the cause of the test failure. (It is minor so I will R+, but please fix so the test passes before landing)

Comment 39

[Dominic Kuo [:dkuo]]

Comment on attachment 8383913 [details] [review]
patch for master

The music part looks good to me and since the media apps will have the same change, I am fine with it.

Comment 40

[Punam Dahiya [:pdahiya]]

Attachment: Patch for master with unit test fixed

This patch has unit tests fixed and is build on patch in attachment 8383913 [details] [review] (comment #39).

Comment 41

[Punam Dahiya [:pdahiya]]

Travis is green for attachment 8387028 [details] [review]. Carrying over the r+ from comment #37, comment #38 to land pr on master.

Comment 42

[Punam Dahiya [:pdahiya]]

Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/a62c8df80732f9b640bfd90add5cdce71a39105c

Comment 43

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

(In reply to Jason Smith [:jsmith] from comment #17)
> (In reply to Hema Koka [:hema] from comment #16)
> > If this bug is in the window management code, perhaps we should first check
> > if it is a easy fix there before we add a workaround (it looks like we are
> > able to reproduce this on Buri too). 
> 
> The window manager bug might be bug 959056. Tim would know for sure though.
> 

My opinion would be "maybe" after reading the comment 0.

Clearing flag here because this bug is fixed and the other one is filed already.

Comment 44

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Marked as 'in-moztrap -' due to this button being removed from all apps that would require a memory card to use