935072 - [B2G][Gallery] Crop handles are difficult to grab and move

Status: RESOLVED FIXED

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

Description

[Matthew Vaughan]

Attachment: Video of difficulty grabbing and moving crop handles

Description:
The crop handles that are displayed around an image in Edit Mode are difficult to move, especially when they are on the border of the image. Most of the time the user can grab and move the handles on the left and right sides of the image. Most of the time it is much more difficult to grab the handles at the top, bottom, and corners of the screen.

Repro Steps:
1) Update Buri to v1.2 COM RIL BuildID: 20131103004003
2) If needed, take a picture using the Camera app
3) Launch the Gallery app
4) Select an image
5) Tap the Edit Mode button (image with a pencil)
6) Tap the Crop button
7) Select any crop option in the toolbar (Freeform has the corner handles)
8) Attempt to grab and move crop handles (especially the top, bottom, and corners)

Actual:
Grabbing and moving crop handles is difficult. A lot of the time the user simple just moves the crop box.

Expected:
Grabbing and moving crop handles is fairly easy.

Environmental Variables:
Device: Buri v1.2 COM RIL
BuildID: 20131103004003
Gaia: cb981e2f47bc644b4d178d54378c3676c946facf
Gecko: eec4da1b27eb
Version: 26.0
RIL Version: 01.01.00.019.276

Notes:
Repro frequency: 4/5
Link to failed test case: https://moztrap.mozilla.org/manage/case/1837/
See attached: CropHandles.3gp

Comment 1

[Jason Smith [:jsmith]]

Does this reproduce on 1.1?

Comment 2

[Matthew Vaughan]

This bug does NOT reproduce on the 11/01/13 1.1 build using COM RIL. The user is able to grab all crop handles, under all crop options, and move them without an issue.

Environmental Variables:
Device: Leo v1.1 COM RIL
BuildID: 20131101041316
Gaia: 39b0203fa9809052c8c4d4332fef03bbaf0426fc
Gecko: 31fa87bfba88
Version: 18.0
RIL Version: 01.01.00.019.276

Comment 3

[Jason Smith [:jsmith]]

Marcia - Can you check to see if you can reproduce this & indicate the user impact of the bug?

Comment 4

[Marcia Knous [:marcia]]

Using Buri and the latest 11-4 base image I definitely see this issue:

Gaia   be4ea00a50236b10eb0a03232a28ffd0048e0cb8
SourceStamp 3ba912717904
BuildID 20131105004003
Version 26.0

There is a marked difference in trying to use the first option (full size) and move the handles compared to the Leo 1.1 shipped device. In my opinion it renders cropping almost unusable in the current state if you use that first option.

I will try this using 1.3 in a moment on the same device.

Comment 5

[Marcia Knous [:marcia]]

I see the same behavior as described in Comment 4 on Buri using the latest 1.3 build (using the same base image):

Gaia   3b5fe429f2414f2a9d7241b311b399033bb10612
SourceStamp 9ba3faa35c96
BuildID 20131106040203
Version 28.0a1

Comment 6

[Matthew Vaughan]

It appears that the crop handles get more difficult to grab and move in the 9/09/13 1.2 build. I would like to note though that it can still be a little difficult to grab and move crop handles on builds before 9/09, just not as bad as after. 

Also on the earlier builds, the white finger contact circle, that appears on the crop handles when a user grabs one, will be present. On builds after 9/09, that white circle will maybe flicker for a quick moment but mostly not be present.

- Works -
Environmental Variables:
Device: Buri v1.2 Mozilla RIL
BuildID: 20130906040204
Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
Gecko: ab5f29823236
Version: 26.0a1

- Broken -
Environmental Variables:
Device: Buri v1.2 Mozilla RIL
BuildID: 20130909114657
Gaia: aa4180e9286d385fa6b62d236f30fb24cd8b93e9
Gecko: 218d4334d29e
Version: 26.0a1

Comment 7

[Marcia Knous [:marcia]]

I retested on Buri using the latest build because I think something relating to the build in Comment 4 wasn't quite right - however, I get the same results:

Gaia   2140c987fdde1c99097018f7e93b0bbd43d2125d
SourceStamp 6a831fcb96f4
BuildID 20131106004004
Version 26.0

Comment 8

[Hema Koka [:hema]]

John, 

Could you please take a look at this?

Thanks!
Hema

Comment 9

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

Sure. I can handle this one.

Comment 10

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

The crop handles uses touch position +/- 25 to check if user touches on it or not. The argument doesn't change since 2012-07-31. I am wondering if this is really a regression. For me, it is not so hard to grab the handles. If changing the value to +/- 30, it is far more easy to grab that.

And as to the white finger contact circle, it looks like a canvas's bug. If we change the color from rgba(255,255,255,.5) to rgb(255,255,255), it shows up.

Comment 11

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

BTW, if we need to change the size to +/- 30, that may also change the minimum size of cropping area.

Comment 12

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

Ok, I found the root cause of missing white finger contact circle. If we disable the line: context.lineJoin = 'round';, it shows up as expected.

Comment 13

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

For the missing white finger contact circle, I had tested with gecko version at 20130906040204 and master gaia. It shows up. That may be a regression from gecko. I will create a bug for this one.

Comment 14

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

I had check the code of gaia between 94e5f269874b02ac0ea796b64ab995fce9efa4b3 and current master. It doesn't have any difference at the cropping feature, including gesture detector.

I had tried different changes on visual to test how people's feel in Taipei office, including enlarging the handle, replacing with circle, and resizing the preview image. Finally, we feel to resize the preview image is better. So, I will upload a patched version with the workaround at comment 12.

Comment 15

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

Attachment: patched version.

Hi Matthew and Marcia,

I can't judge the word of "difficulty grabbing". Please test this version and give some feedback.

This version doesn't change the crop handle size but resize the preview image. The  edge between preview image and header is larger than before.

Comment 16

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

Sorry, I forgot to mention that the zip file is the gallery app which is the same as application.zip. Please install to your device. You may use adb push to the position of original gallery app and restart the gallery app.

I feel this bug is hard to tell how difficulty to grab the crop handles. That's the reason I need your helps to tell me if this version is difficulty to grab.

Comment 17

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

Attachment: the edge between header and preview image is 20px

This is the screenshot of patched version. The original offset 10px. I had enlarged to 20px. That's far more easy to grab the north and south handles.

Comment 18

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

I create another bug 937529 for hosting missing active handle bug.

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

The OEM Build shows this issue still; not sure but part of it seems to be with the allowance and positioning?

Gaia   2ef9bc3c7a6de228b63e6ba3613eb0c0dd639c59
BuildID 20131105151351
Version 26.0a2
1.2 Buri

Will test using blobs with our 1.2 gecko/gaia as soon as I'm done testing the OEM build

Comment 20

[Matthew Vaughan]

Hello John.

I tested your patch for the Gallery in regards to the crop handles, and I feel that it is quite a bit easier to grab the handles when the image is slightly smaller. There is an issue when grabbing the upper corner handles however, which is the user can sometimes tap the Cancel or Save buttons when they try to grab the upper corner handles. It seems like the Cancel and Save buttons are larger than they appear. I will write up a separate bug to track this issue.

Comment 21

[Jason Smith [:jsmith]]

(In reply to Matthew Vaughan from comment #20)
> Hello John.
> 
> I tested your patch for the Gallery in regards to the crop handles, and I
> feel that it is quite a bit easier to grab the handles when the image is
> slightly smaller. There is an issue when grabbing the upper corner handles
> however, which is the user can sometimes tap the Cancel or Save buttons when
> they try to grab the upper corner handles. It seems like the Cancel and Save
> buttons are larger than they appear. I will write up a separate bug to track
> this issue.

Is the issue you are seeing from this patch or not? If it's from this patch, then you should not file a separate bug for this.

Comment 22

[Matthew Vaughan]

Sorry for not being clear about the issue I saw, but it occurs outside the patch, i.e. it occurs on the regular build. I created bug 937806 to track this issue.

Comment 23

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I haven't tested the patch; with the gaia/gecko and the OEM 1.2; this seems better than before.  Having said that some tweaks would make it a lot better.  I encountered errors when trying to reposition and also going to the frame ratio.

Gaia:     377eb71506d8790d3c3280d82ac007ff4525b7e0
Gecko:    c15f5d7e0d7ebff5f8edef0745f144cd1552e34c
BuildID   20131112123250
Version   28.0a1
Base Build : US_V1.2_20131111.cfg
Buri

Comment 24

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

Attachment: overflowing the master on v1.1

I feel I found the root cause of this one. The attachment is to put two screenshot at the same layer. We may found the master has larger header and toolbar. They eat some area of the space between handles and them. I will put another horizontal aligning comparison.

Comment 25

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

Attachment: horizontal alignment

Comment 26

[Jason Smith [:jsmith]]

Removing qawanted per comment 20's testing.

Comment 27

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

The difference between v1.1 and v1.2~(and master) are:
1. space of top: 3px (master - v1.1,v1.2) ==> a progress bar which is landed recently, so we don't have this in v1.2.
2. space of bottom: 5px (v1.2~ - v1.1) ==> a UI polish which is landed at 10th Sept to v1.2.

They may also cause the difficult grabbing problem. Our touch handles are 25px in width/height. If the spaces on each edge are larger than 25px, it is very easy to grab them. In the design of v1.1, we leave top/bottom is 10px which is relative easy to grab. But in master, there is a progressbar at the top which consumes 3px and toolbar at bottom(v1.2) is 5px larger. I also found a small size issue that we leave 5px(1.2~)/10px(1.1) empty between the bottom toolbar and grabbing area. If we remove the extra space between bottom bar and grabbing area, it will be more easy to grab.

Comment 28

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

Attachment: recycle and get more size for cropping handles

The changes are:
1. hide the saving progress bar by default and show it when needed and hide it when saved.

2. get back 5px from edit-options and recycle 5px between edit-preview-area and edit-options. The 5px of edit-options is introduced at bug 800002. But I can't find any spec says we need to enlarge edit-options 5px.

3. scale the preview image down to have 15px on each edge to have larger space for handles.

4. change the border of selected options from 3px to 2px. There is a 1px calculation offset in 1.5x device. The horizontal offset is already existed before this patch. But when we reduce the height of edit-options from 55px to 50px, the vertical offset shows and that makes horizontal/vertical offset more noticeable to user.

Comment 29

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

Comment on attachment 831385 [details] [review]
recycle and get more size for cropping handles

One more thing:
5. introduce the workaround for bug 937529 to eliminate the dependency.

Comment 30

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

Attachment: screenshot after patched.

Comment 31

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

Attachment: comparison between patched and original

The bottom image is the patched screenshot and semi-transparent image is the original screenshot.

Comment 32

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

Attachment: bug935072.zip

Rob,

Please help us to review this patch. The screenshot can be found at attachment 831389 [details] and comparison between original version can be found at attachment 831407 [details].

Comment 33

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

Comment on attachment 831408 [details]
bug935072.zip

Matthew,

Please help us to test this version to check if handles is easy to grab. And please use 1.3 gecko to test with this because of bug 937806.

Comment 34

[Matthew Vaughan]

John,

Your patch made the grabbing of the crop handles pretty easy in 1.3, especially when comparing it to how it currently is on the build. The resizing of the image and the UI doesn't hinder the user in any way either. I had a couple other testers try out the patch as well and, when compared to how it is grabbing the crop handles now, they noticed a good bit of improvement. There might be a little bit of difficulty for some users even with the patch, but I honestly think it might be the quality of the touch screen on the Buri devices causing some problems. Overall, we think that this patch makes grabbing the crop handles a decent amount easier.

Comment 35

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

Thanks Matthew.

BTW, this version doesn't resize the image on top and bottom. I just recycle the useless 5px and reduce 5px from edit options. If you see the image at attachment 831407 [details], you may find the image is only moved 5px down.

Comment 36

[David Flanagan [:djf]]

Comment on attachment 831385 [details] [review]
recycle and get more size for cropping handles

The code all looks good to me. If UX is okay with the visual changes, this seems like a great fix. 

I have a few comments on github, including a typo to fix. 

Also, if the progress bar is not already in the 1.2 branch it probably should be because it was added for a 1.1hd bug. So if it is not there, consider landing the image saving fix to 1.2 first before landing this one.

Comment 37

[Jacqueline Savory [:jsavory] UX]

Rob asked me to fill in for a UX review of this patch. 
I think that creating more space between the navigational elements and the crop area is very effective and I find it quite a bit easier to tap on the handles without accidentally selecting other options, so I am happy with the changes. 

However I think we can further improve this issue by altering the hit area of the crop handles a little. Currently I find that the best way to get a hold of them is to aim just outside the crop handle, if the area of selection is expanded to include a little more of the area inside the crop box, it might help users to select them on the first try, especially for the corners. This, however, could be done at a later time and may not be necessary for this particular bug.

Comment 38

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

merged to master:

https://github.com/mozilla-b2g/gaia/commit/8d4f461310c53478a0cdc44ecb8ad10fdb647d66

I will need to check the code of v1.1hd and v1.2 to determine how to uplift this patch to v1.1hd and v1.2.

Comment 39

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

landed to v1.1.0hd:

https://github.com/mozilla-b2g/gaia/commit/6e46d4d247f2ee15b79193e2f8368d3976b930d2

Comment 40

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

landed to v1.2:
https://github.com/mozilla-b2g/gaia/commit/a6484b1e6fc07cf6bd8d6fcf9aeebb14b7e8869d

The patch of progress bar had landed to v1.2. So, it's easy to uplift this to v1.2.

Comment 41

[Jason Smith [:jsmith]]

(In reply to John Hu [:johnhu] from comment #39)
> landed to v1.1.0hd:
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 6e46d4d247f2ee15b79193e2f8368d3976b930d2

Why was this landed to 1.1 HD? This doesn't have a hd+.

Comment 42

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

Sorry, Jason.

I am wrong at this action. We should back it out.

I land it just because the patch of progress bar was landed to 1.1hd. So, I feel I need to do it and forgot this is only a koi+ bug. Sorry about that. I will back it out....

Comment 43

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

The patch for v1.1.0hd is backed out:

https://github.com/mozilla-b2g/gaia/commit/0da6c5d4cb2e2e0e22a0402c36fdf765c484c906