1039988 - [Gallery] Share button is not working

Status: VERIFIED FIXED

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

Description

[ranjith253]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.153 Safari/537.36

Steps to reproduce:

 1. Make sure that gallery has a image with EXIF orientation data , probably taken from    camera app.

2. In full screen view click share button.



Actual results:

Share options are not listed. Following error is thrown when share button is clicked

[JavaScript Error: "TypeError: button is null" {file: "app://gallery.gaiamobile.org/js/frame_scripts.js" line: 2280}]


Expected results:

Share options has to be listed .

Comment 1

[ranjith253]

Attachment: PR

Hi Punam,

Review the attached PR for this bug.

Comment 2

[Punam Dahiya [:pdahiya]]

This issue is replicable for images that needs EXIF data to rotate and display correctly (e.g. images taken from Tarako device).Attaching sample image to replicate the issue

Comment 3

[Punam Dahiya [:pdahiya]]

Attachment: Sample image to replicate bug 1039988

Comment 4

[Punam Dahiya [:pdahiya]]

Comment on attachment 8462505 [details]
PR

Thanks Ranjith for the patch, r+. It's a miss from bug 1002593 in which we imported 1.3T downsampling changes into master. 

Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/29984810ebbe22d65e08d51b1728620aac74463b

Comment 5

[Punam Dahiya [:pdahiya]]

1002593 changes were done in 1.4 and 2.0, so this issue should exist in 1.4 and 2.0. Setting NI flag for Hema to decide and help uplift this fix to 1.4 and 2.0. Thanks

Comment 6

[Hema Koka [:hema]]

Adding qawanted to confirm if this is a regression issue on 1.4 and 2.0?

Thanks
Hema

Comment 7

[Chris Kreinbring [:CKreinbring]]

The bug repros on Flame 2.0 and Flame 1.4
Actual result:  Tapping the Share icon while viewing the attached picture in full screen will not bring up the Share menu.

Flame 2.0
BuildID: 20140910075715
Gaia: 3f4c635106c5364228782d12b1cb76b0c105b971
Gecko: 0b64a36a0378
Platform Version: 32.0
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Flame 1.4
BuildID: 20140910070823
Gaia: 6018a1c18f0c3eab25aac2ba3064904740591dd2
Gecko: 39c9dbc311f7
Platform Version: 30.0
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 8

[David Flanagan [:djf]]

Yes, this is a regresssion. Between porting to the tablet form factor and working to handle EXIF orientation we broke this. In 1.3 and before you could share images that had exif orientation (they wouldn't be rotated before being shared, but they would be shared). Now, with this bug, images that need to be rotated don't get shared at all

Comment 9

[David Flanagan [:djf]]

Comment on attachment 8462505 [details]
PR

This bug came up in the discussion and testing of 1.4+ bug 1065877, and since we are uplifting that one, I think we should uplift this one, too.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): This is a regression caused by porting to the tablet form factor at the same time, but out of sync with, the work to support exif orientations.


[User impact] if declined: users will not be able to share images that use EXIF orientation. Users may receive these images by email, sms, or bluetooth, but won't be able to pass them on, and the share button the gallery will be broken.

[Testing completed]: yes

[Risk to taking this patch] (and alternatives if risky): Completely risk free. It is a one-line patch. The current line is guaranteed to throw an exception, so there is no way that replacing that line with a new line of code could make anything worse. 

[String changes made]: none

Comment 10

[David Flanagan [:djf]]

Comment on attachment 8462505 [details]
PR

Switching the approval requests to Bhavana. See the comment above, please. I think this is a completely risk-free uplift that will solve a 1.4 bug that spreadtrum identified independently in bug 1065877.

Comment 12

[David Flanagan [:djf]]

Duplicate bug 1069987 was reported against 2.0, so this issue has now been independently found and reported in both 1.4 and 2.0. Given that it is a trivial patch, I think we need to uplift.

Comment 13

[Tony Chung [:tchung]]

Kevin, please have someone verify this patch is fixed on 2.0 and master after its been landed and uplifted.   I find the easiest way is to download a jpg from the internet, and view it in gallery, then ensure the share button loads the list and connects to the web activity app.   Thanks.

Comment 14

[bhavana bajaj [:bajaj]]

Comment on attachment 8462505 [details]
PR

Blocking 1.4 given the user impact.

I am approving this for 1.4/2.0 given the low risk. In parallel I am trying to work with QA to understand how we missed this.

QA please make sure to test/verify once this lands on 2.0.

Comment 15

[Tony Chung [:tchung]]

(In reply to bhavana bajaj [:bajaj] from comment #14)
> Comment on attachment 8462505 [details]
> PR
> 
> Blocking 1.4 given the user impact.
> 
> I am approving this for 1.4/2.0 given the low risk. In parallel I am trying
> to work with QA to understand how we missed this.

I investigated this myself, and it actually seems a bit edge-casey.  For the attachment in this bug, I can reproduce the missing sharing button.  but if i took my own picture from the camera app, the sharing options were available as expected.

I took a screencast of the two scenarios:  http://youtu.be/0G3u0AJMZe8

Alternately, there are sharing tests in the regression repository, but they werent reproducing this issue.   The tests i found has to do more specifically with doing multi-image selection, sharing attachments via email, and sharing via twitter 3rd party app.  There was also a similar bug filed 10 months back by marcia that was marked as WFM. (see bug 1039988)

Which is why i conclude this is a test that caught an edge case, and we have already put plans into creating a regression test for this.   

flagging in-moztrap=? Ktucker

> 
> QA please make sure to test/verify once this lands on 2.0.

Comment 16

[David Flanagan [:djf]]

Tony and Kevin,

This bug only affects images that use EXIF rotation. See comment 9. So it is an edge case, but given that it has now been reported independently in 1.4 and 2.0, its apparently not such an uncommon edge case. I think there are enough cameras out there that use EXIF rotation that we need to be careful to support it correctly.

I have filed bug 1070207 to create a set of manual test images that we can use to help ensure that things like this don't fall through the cracks in the future.

Comment 17

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

Test case has been added to moztrap here:

https://moztrap.mozilla.org/manage/case/14682/

Comment 18

[Tony Chung [:tchung]]

(In reply to David Flanagan [:djf] from comment #16)
> Tony and Kevin,
> 
> This bug only affects images that use EXIF rotation. See comment 9. So it is
> an edge case, but given that it has now been reported independently in 1.4
> and 2.0, its apparently not such an uncommon edge case. I think there are
> enough cameras out there that use EXIF rotation that we need to be careful
> to support it correctly.
> 
> I have filed bug 1070207 to create a set of manual test images that we can
> use to help ensure that things like this don't fall through the cracks in
> the future.

Thanks for the tips!  will follow up over in that bug.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/d8654c286c40ef5d828a338b7922de5bea1d23ec
v1.4: https://github.com/mozilla-b2g/gaia/commit/cf84c8db5ba472abd35b5b43b128c1aa70a759e6

Comment 20

[Kai-Zhen Li [:seinlin][:kli]]

v2.0m: https://github.com/mozilla-b2g/gaia/commit/d8654c286c40ef5d828a338b7922de5bea1d23ec

Comment 21

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

Yeojin, please verify this issue.

Comment 22

[Yeojin Chung [:YeojinC]]

The Share menu is displayed properly on Flame 2.1 KK, Flame 2.0 KK, Flame 2.0 JB, Flame 1.4 JB:

Flame 2.1 KitKat Base (319mb)

Environmental Variables:
Device: Flame 2.1
BuildID: 20140923003005
Gaia: 3742913e11f69e789dcb0aa0dedf2e5572da0129
Gecko: df42b05782aa
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0 KitKat Base (319mb)

Environmental Variables:
Device: Flame 2.0
Build ID: 20140923063014
Gaia: 6449cc35a8f0704d95acac374ba857bde4b86d6c
Gecko: b930730dba81
Version: 32.0 (2.0)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Flame 2.0 Jelly Bean Base (319mb)

Environmental Variables:
Device: Flame 2.0
Build ID: 20140923000205
Gaia: 8d7f2ac85f3154bdb149d67e5c2f9b035f5e4105
Gecko: 6dd19beda1c2
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Flame 1.4 Jelly Bean Base (319mb)

Environmental Variables:
Device: Flame 1.4
BuildID: 20140923000203
Gaia: 19772b10ec6f127ed8574c3ef5e022a88c85fd5e
Gecko: e573f292783f
Version: 30.0 (1.4) 
Firmware Version: L1TC00011230
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 23

[Coler]

Verify passed, this issue can't be repro on Woodduck 2.0.
Attached: Verify_Woodduck_Sharebutton.MP4
Reproducing rate: 0/5
Build version:
Gaia-Rev        7bc7f75d712ccef535fd371bfcc7fe61dcdcf874
Gecko-Rev       8f21e6d8abf8ee01d8da066495d8febf3138375a
Build-ID        20141113050313
Version         32.0

Comment 24

[Coler]

Attachment: Verify_Woodduck_Sharebutton.MP4

Comment 25

[Julien Wajsberg [:julienw]]

Josh, please, same issue here, I don't want that bugs that have patches stay confidential...
Thanks!

Comment 26

[Josh Cheng [:josh]]

Hi Coler,
This is not reported by Woodduck partner. No need to make this Woodduck confidential. Thanks!