4.21 MB, video/3gpp
166.94 KB, image/png
247.01 KB, image/png
46 bytes, text/x-github-pull-request
|Details | Review|
151.95 KB, image/png
181.02 KB, image/png
482.19 KB, application/zip
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
Does this reproduce on 1.1?
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
Marcia - Can you check to see if you can reproduce this & indicate the user impact of the bug?
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.
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
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
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
John, Could you please take a look at this? Thanks! Hema
Assignee: nobody → johu
Sure. I can handle this one.
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.
BTW, if we need to change the size to +/- 30, that may also change the minimum size of cropping area.
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.
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.
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.
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.
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.
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.
I create another bug 937529 for hosting missing active handle bug.
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
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.
(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.
Attachment #830656 - Flags: feedback?(mvaughan) → feedback+
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.
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
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.
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.
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.
Attachment #831385 - Flags: review?(dflanagan)
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.
Attachment #831266 - Attachment description: overflowing the v1.1 on master → overflowing the master on v1.1
The bottom image is the patched screenshot and semi-transparent image is the original screenshot.
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 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.
Attachment #831408 - Flags: feedback?(mvaughan)
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.
Attachment #831408 - Flags: feedback?(mvaughan) → feedback+
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 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.
Attachment #831385 - Flags: review?(dflanagan) → review+
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.
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.
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.
(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+.
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....
The patch for v1.1.0hd is backed out: https://github.com/mozilla-b2g/gaia/commit/0da6c5d4cb2e2e0e22a0402c36fdf765c484c906
You need to log in before you can comment on or make changes to this bug.