Closed Bug 1020766 Opened 10 years ago Closed 10 years ago

[Flatfish][Gallery] Gallery app shows last-deleted photo in the preview window

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jsavage, Unassigned)

Details

(Whiteboard: [Flatfish][TCP])

Attachments

(2 files)

I've been able to delete one photo at a time with no issues until I get to the very last one. 

Steps to reproduce:
1. Take six photos with the Camera app.
2. Go to the Gallery app. 
3. Tap on any of the photo to open it. Tap on the delete icon.
4. Repeat steps 2-3 on each of the other photos (one at a time) until all the photos are gone.

Expected:
When I delete a photo, it should disappear from the Gallery and the Preview pane.

Actual result:
Then, even after tapping "Delete" in the confirmation window, that last photo still appears in the preview pane but not in the gallery. When I double-tap on the preview pane, it zooms in as if the photo's still there. I have to reboot the device to make the photo disappear.

I tried this three times with the same result: first with 1 photo, then 3, then 6. It always happens with the last photo in the Gallery.
Summary: [Flatfish}[Gallery] Gallery app crashes when I delete the last photo in the Gallery → [Flatfish}[Gallery] Gallery app crashes when I delete the last photo
When does the app officially crash? Have you tried closing the app (2 finger swipe up to get the app switcher) vs. rebooting the device? There's probably nothing available to show at this moment when the last picture is deleted.
Summary: [Flatfish}[Gallery] Gallery app crashes when I delete the last photo → [Flatfish][Gallery] Gallery app crashes when last photo is deleted
Whiteboard: [TCP] → [Flatfish][TCP]
Is your wifi turned on and did you accept the crash report sending?  Did you also get the crash report for sending?

If so you can find your crash report here : 
https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#command_line_to_get_the_crash_ids
and that could help us understand what the crash was.
Andrew and Naoki, I tried it again. The app doesn't officially crash, so there isn't a crash report to send. I'll update this bug title.

Andrew, I closed the app using the 2 finger swipe and then relaunched it. The last pic I attempted to delete is still showing up in the preview window, but there's nothing in the gallery.
Summary: [Flatfish][Gallery] Gallery app crashes when last photo is deleted → [Flatfish][Gallery] Gallery app shows last-deleted photo in the preview window
Hi,
I can reproduce the bug and interested in it,may i take it?
Flags: needinfo?(feer56)
I can make that call. Asa or Naoki, I'm not sure if we are doing fixes for the TCP program yet or if this affects other Flatfish devices. Could you direct this bug to the appropriate people so they can decide whether "Andrew Zhi" can take a stab at it? Thanks.
Flags: needinfo?(nhirata.bugzilla)
Flags: needinfo?(feer56)
Flags: needinfo?(asa)
Attached file debugger error message
Model:flatfish
Os Version:2.1.0.0-prerelease
gaia commit id:4e10f046

when deleted last photo in LAYOUT_MODE:'fullscreen' or LAYOUT_MODE:'thumbnailSelecView',
"MediaDB: " "ondeleted" "event handler threw" TypeError: files[currentFileIndex] is undefined
detail:https://bugzilla.mozilla.org/attachment.cgi?id=8444289
so ,didn't to run -> showOverlay('emptygallery')@app://gallery.gaiamobile.org/js/gallery.js:549
This doesn't seem to occur on the flame device that I can tell.  Maybe I might be doing something wrong.

How do you transition from the Camera to Gallery?  ie do you hit the home button and then go to gallery?  Do you also have other pictures already stored on the device?
Flags: needinfo?(nhirata.bugzilla)
i believe that it doesn't occur on the flame device:
debug : "MediaDB: " "ondeleted" "event handler threw" TypeError: files[currentFileIndex] is undefined
Trace code:
dispatchEvent@gaia/shared/js/mediadb.js:2006
initDB/photodb.ondeleted@gaia/apps/gallery/js/gallery.js:348
fileDeleted@gaia/apps/gallery/js/gallery.js:507
    -> files:array is empty,after deleted last file.
updateFocusThumbnail()@gaia/apps/gallery/js/gallery.js:1079
    -> if (isPhone) return; -> (line:1082),that's why it doesn't occur on the flame device.
    -> files.length==0 -> files[currentFileIndex] is undefined

Debug:
there are 2 ways to delete file,and then it will go to call updateFocusThumbnail():

LAYOUT_MODE.fullscreen:
	-> fileDeleted():updateFocusThumbnail()@gallery/js/gallery.js:542
LAYOUT_MODE.select:
	-> setView()@gallery/js/gallery.js:542
	-> setView():showFile()@gallery/js/gallery.js:667
	-> showFile():updateFocusThumbnail()@gallery/js/frames.js:467

Expected:
if no TypeError,it will go on showOverlay('emptygallery')@gaia/apps/gallery/js/gallery.js:549
like phone ux -> after deleted last photo that means no photo in gallery, -> popup to ask user if launch to camera app
Flags: needinfo?(nhirata.bugzilla)
Personally I don't see any harm if you want to try to create a patch for it.  I think :djf might be the person to ask for reviewing the patch.
Flags: needinfo?(nhirata.bugzilla)
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #10)
> Personally I don't see any harm if you want to try to create a patch for it.
> I think :djf might be the person to ask for reviewing the patch.
i got it,i will try to fix it,thanks for your help.
Attachment #8448705 - Flags: review?(dflanagan)
Comment on attachment 8448705 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/21211

Andrew,

Thank you for fixing this bug! I have given r- because I'd like you to change the comments in your code. The code itself seems fine, though.  Please see my comments on github. When you have updated the pull request request another review from pdahiya@mozilla.com. She can do a final review and land the patch for you.

Note that I have not actually tested this patch on a flatfish myself, but am satisified that your changes only affect the tablet and cannot affect the app operation on a phone.
Attachment #8448705 - Flags: review?(dflanagan) → review-
Attachment #8448705 - Flags: review?(pdahiya)
Hi David ,
I have learned a lot from your comments on github.
Thank you for your advice.

Hi Punam ,
I have updated the pull request.Could you review it ?
Thank you!
Comment on attachment 8448705 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/21211

Patch looks good, i have left couple of comments mostly nits on github, with those updated we can land the patch on master. Thanks
Attachment #8448705 - Flags: review?(pdahiya) → review+
updated the pull request:https://github.com/mozilla-b2g/gaia/pull/21211
Thank you for your review.
Flags: needinfo?(pdahiya)
Hi Andrew

There's lint error that's causing travis job to fail. Please fix. Thanks
Flags: needinfo?(pdahiya)
Hi Punam Dahiya,

have fixed comment (line:666)
https://github.com/mozilla-b2g/gaia/pull/21211
Thank you!
Flags: needinfo?(pdahiya)
Thanks Andrew, patch landed on master

https://github.com/mozilla-b2g/gaia/commit/99f56d9db3cd37c684b01de6fed786421f47e2b7
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Clearing NI flag
Flags: needinfo?(asa)
Flags: needinfo?(pdahiya)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: