1149183 - [Gallery][Edit] Unable to manually adjust the cropping frame.

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Martin Shuman [:Marty]]

Attachment: logcat-picture-crop.txt

Description:
When editing a picture, the user is unable to manually move the cropping frame.

-The user is still able to use the various cropping presets, like 3:2 and 1:1.

-This issue also occurs when attaching a picture to an MMS


Repro Steps:
1) Update a Flame to 20150330010204
2) Open the Gallery
3) Select a Picture and tap the Edit button
4) Select the Crop tool and attempt to manually move the cropping frame


Actual:
The Cropping frame cannot be selected, moved, or resized.


Expected:
The cropping frame can be selected, moved, and resized.

Environmental Variables:
Device: Flame 3.0 (319MB)(Full Flash)
Build ID: 20150330010204
Gaia: be25b16efa19bab8d54be08f8fe45dcc93bf93d0
Gecko: dfe60814eda7
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Repro frequency: 7/7
Link to failed test case: https://moztrap.mozilla.org/manage/case/10742/
See attached: Video (URL), Logcat

Comment 1

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Functional regression that fails a smoke test.

Requesting a window.

Comment 2

[Martin Shuman [:Marty]]

This issue does NOT occur on Flame 2.2, nor on Friday's Nightly Flame 3.0 build.
The cropping frame can be selected, moved, and resized.

Environmental Variables:
Device: Flame 2.2 (319MB)(Full Flash)
Build ID: 20150330002503
Gaia: 473cd63f53c855299b719285d9b95e3f2910782f
Gecko: 4b13c4254e2f
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 3.0 (319MB)(Full Flash)
Build ID: 20150327010205
Gaia: 249b8c08c1d57961ef6c905f3498fa62b032bf24
Gecko: e046475a75cb
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Comment 3

[Peter Bylenga [:PBylenga]]

After further consideration going to downgrade from smoke, in the past when cropping failed the Smoke Tests, the app would close or the MMS would not be sent.  In this case we're not blocked from completing our flow (send MMS with a picture).

Still nominating to block release.

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1147249 or Bug 1075670 may be the cause of this issue.

Mozilla-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150326141446
Gaia: 525c341254e08f07f90da57a4d1cd5971a3cc668
Gecko: c297ede37ac5
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150326141946
Gaia: 525c341254e08f07f90da57a4d1cd5971a3cc668
Gecko: fb063ad596fd
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 525c341254e08f07f90da57a4d1cd5971a3cc668
Gecko: fb063ad596fd

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 525c341254e08f07f90da57a4d1cd5971a3cc668
Gecko: c297ede37ac5

Gecko Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c297ede37ac5&tochange=fb063ad596fd

Comment 5

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

David and Bill, can you take a look at this please? Unfortunately, we are unsure if the culprit here is bug 1147249 or bug 1075670. We will need the commit that is the cause to be backed out.

Comment 6

[David Flanagan [:djf]]

I suspect it is bug 1075670. The gallery app uses screenX and screenY. Perhaps gallery is using them incorrectly and having them fixed broke this.  I'll investigate whether we can just fix this in gallery instead of backing out the gecko fix.  (If the gecko fix is actually good.)

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Thanks David! Let me know if it turns out we need to back out bug bug 1075670.

Comment 8

[David Flanagan [:djf]]

Bill: yes, we need to back out bug 1075670. I've updated that bug with my findings, but don't know how to do a gecko backout myself.

Comment 9

[David Flanagan [:djf]]

So it seems certain that this is a regression caused by bug 1075670. Bill says he will back that out as soon as the gecko tree re-opens.  I don't know if we need to keep this bug open or not, but I'm going to unassign myself now.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: Ensure we send chrome offset changes to the TabChild

Comment 11

[Kartikaya Gupta (email:[e-mail])]

^ Patch has wrong bug number, corrected locally.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8586144 [details] [diff] [review]
Ensure we send chrome offset changes to the TabChild

The rect should have default ctor to initialize to 0,0

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Try push for sanity check: https://treeherder.mozilla.org/#/jobs?repo=try&revision=263f754e1b07

Comment 14

[No-Jun Park [:njpark]]

I tested it on the latest mozilla-inbound which contains the backout (not Comment 12), and this bug no longer reproduces.

Version:
Build ID               20150331101206
Gaia Revision          1d14f4a6809de81ebd638117ed4ddd3b1b18f033
Gaia Date              2015-03-31 08:49:13
Gecko Revision         https://hg.mozilla.org/integration/mozilla-inbound/rev/c39ec40cb4e1
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150331.100035
Firmware Date          Tue Mar 31 10:00:44 EDT 2015
Bootloader             L1TC000118D0

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Erm, crap. The try run is green, but this patch doesn't really apply with bug 1075670 backed out. David/Bill, do you just want to move this patch over to bug 1075670 and land it on top of the re-landing of all those patches (Or just steal it and squash it into those patches)?

Comment 16

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Yeah, let's just squash it in there. The other patches aren't really correct without it, as I understand the situation. Thanks for all the help!

Comment 17

[Kartikaya Gupta (email:[e-mail])]

No problem. I'll leave it to you guys to do the squashing and landing then.