958681 - [B2G][Gaia][Gallery] When a corrupt image is placed on the device the gallery app can appear to continue loading indefinitely with no warning.

Status: RESOLVED FIXED

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

Description

[Lionel Mauritson]

Attachment: bad2.png

Placing a corrupt image file (in this case corrupted via a hex editor) on to the SD card and then loading the Gallery app can cause the loading bar at the top of the screen to run indefinitely.

Repro Steps:
1) Update a Buri to BuildID: 20140110004009
2) Enable USB on the device.
3) Place a corrupted png file on the SD card (example included)
4) Disconnect USB and load the Gallery app
5) Observe the loading bar at the top of the screen running.

Actual:
The device will try to load the corrupt file indefinitely.

Expected:
The device will recognize the corrupt file and inform the user that it cannot be loaded with a broken image icon.

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140110004009
Gaia: c606b129a2d1647c0fc7bfb197555026e9b27f9e
Gecko: c5109884ae3a
Version: 28.0a2
Firmware Version: 20131115

1.2 Environmental Variables:
Device: Buri 1.2 MOZ
BuildID: 20140108004002
Gaia: 539a25e1887b902b8b25038c547048e691bd97f6
Gecko: 0d8b879ffd70
Version: 26.0
Firmware Version: 20131115

1.1 Environmental Variables:
Device: Buri 1.1 MOZ
BuildID: 20140108041200
Gaia: 6ff3a607f873320d00cb036fa76117f6fadd010f
Gecko: e8c68b175abb
Version: 18.0
Firmware Version: 20131115


Notes: 1.1 Does not show the loading bar, but does not show a corrupt image file indicator either.

Repro frequency: 100%

See attached: Example corrupt image

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

We can load the image offscreen but cannot draw the image to canvas[1]. When we use img element to load the image and attach to body, it fires error event. This may because of the deferring of image decoding if the image is not visible to user.

[1] https://github.com/mozilla-b2g/gaia/blob/9358e73279fc69fb9a126e847ee1f4fce4d3a3f7/apps/gallery/js/MetadataParser.js#L264-L274

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

(In reply to Lionel Mauritson from comment #0)
> Expected:
> The device will recognize the corrupt file and inform the user that it
> cannot be loaded with a broken image icon. 

I think to show the unsupported files is a feature instead of a bug. So, I prefer to make metadata parser more robust and file another bug to request a feature to inform user about the unsupported files.

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: use try catch to catch the error inside createThumbnailFromElement

use try catch to catch the error inside createThumbnailFromElement.

Comment 4

[David Flanagan [:djf]]

Setting the regression flag based on John's analysis in comment #1. I know we used to handle corrupt images correctly because they caused an error when loading the image. In this case it sounds like the image's onload handler fires instead of its onerror hander because decoding of the image is deferred.

Since it is a regression, I'm proposing that we uplift the fix into 1.3.

Comment 5

[David Flanagan [:djf]]

Comment on attachment 8359117 [details] [review]
use try catch to catch the error inside createThumbnailFromElement

Nice patch, John. Thank you!

Do you think that any of our other apps will be affected by this regression?

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

There are three apps having similar code to generate image:
1. For camera app, we should be able to load and parse metadata because those photos are created by us.
2. For video app, we don't have similar issue here because the decoding sequence of gecko is different and it already have a try-catch to handle it.
3. For wallpaper app, we don't need to handle it because all wallpapers should be decodable.

So, this seems the last case here and hope that's the last case.

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/fc4a1df95b6c2148193dcb991f30cb130a9bbbf7

Comment 8

[Preeti Raghunath(:Preeti)]

Please request approval flag here.