Closed Bug 826211 Opened 11 years ago Closed 11 years ago

[Gallery] Crop box border line not display perfectly

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

(b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

RESOLVED FIXED
Tracking Status
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: leo.bugzilla.gaia)

References

Details

Attachments

(3 files, 1 obsolete file)

1. Title : Crop box border line not display perfectly
2. Precondition : 
3. Tester's Action : Settings - Display - Wallpaper - Gallery - any image - crop
4. Detailed Symptom (ENG.) : Not display bottom line of crop box
5. Detailed Symptom (KOR.) : 
6. Expected : Crop box border line should be displayed perfectly
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :

2. Precondition : 
3. Tester's Action : 
4. Detailed Symptom (ENG.) : 
5. Detailed Symptom (KOR.) : 
6. Expected : 
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :
This issue shows up when the gallery image chosen is a portrait picture. The bottom crop line is being displayed, its just off the bottom of the screen. If you pull down the top line, you can then drag the crop rectangle up and see the bottom crop line.

The real issue is the bottom of the image is off the bottom of the screen. The image should be resized so it fits both horizontally and vertically in the visible space.
The positioning of the crop overlay is not proper in leo. modified the code.
please review it.

Pointer to the pull request.
https://github.com/mozilla-b2g/gaia/pull/8120
I think David would be an appropriate reviewer here.
Attachment #714937 - Flags: review?(dflanagan)
Comment on attachment 714937 [details]
Pointer to Github Pull Request: https://github.com/mozilla-b2g/gaia/pull/8120

I can't reproduce the bug in the first place, and I can't see any difference in the crop box with the proposed change, so I can't give r+ for this patch.

Also, the proposed change changes the permission bits on the css file to make it executable, which we don't want.

Attaching a screenshot of the bug would be helpful. Also, if the bug is being seen in a leo-specific branch, it would be helpful to know where that branch is.
Attachment #714937 - Flags: review?(dflanagan) → review-
Assignee: nobody → leo.bugzilla.gaia
Attached image screenshot_1
attachment one which shows "Crop box border line not display perfectly"
Attachment #716476 - Flags: review?(dflanagan)
Attached image screenshot_2
attachment two fofr "Crop box border line not display perfectly"
Attachment #716477 - Flags: review?(dflanagan)
Bug reproducible in "gaia" revision="04e3e923bdae0773b0133ffa4958831b7124bf7a"
I didn't read the steps to reproduce carefully enough. I now see that this is for cropping as part of the Pick activity, not the normal crop while editing a photo.  My mistake.

I suspect that there is a better fix that would involve a JavaScript change, and will investigate.
I've spent a long time trying to understand what regressed this...  But it looks like this bug has been here forever, and I never noticed it, and wasn't cc'ed on the bug report until this week. Ugh.
Comment on attachment 714937 [details]
Pointer to Github Pull Request: https://github.com/mozilla-b2g/gaia/pull/8120

Now that I understand this bug correctly (Sorry!) I'm changing my review to r+ if you make one change: instead of using height: calc(100%-50px) please use the simpler: bottom: 0px; That way we only have to have the constant 50 in one place.

This fixes the bug illustrated in the attached screenshot 2.  But I don't think it fixes the bug in screenshot 1.  I can't reproduce that, but am worried about it.  If you know how to reproduce it, please file another bug and cc me or assign to me.
Attachment #714937 - Flags: review- → review+
Attachment #716476 - Flags: review?(dflanagan)
Attachment #716477 - Flags: review?(dflanagan)
On second thought, I suspect that the bug assignee does not have commit rights, so I've gone ahead and made the change I suggested in my review and have landed this one-line CSS change on gaia master:

https://github.com/mozilla-b2g/gaia/commit/99ebe8ba7417e73ca0acf9f110ef0d781a80989e

This is an easy fix for a semi-serious bug, and I think we ought to get it on the v1-train, so I'll nominate it.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This is the slightly tweaked version of the proposed patch that I just landed on master. We should get this on the v1 train, so I'm requesting approval-gaia-v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): implemented wrong from the beginning, it appears.
User impact if declined: cropping portrait mode images from the gallery pick activity will cut off the bottom of the image. 
Testing completed: yes, locally
Risk to taking this patch (and alternatives if risky): not risky; one-line CSS change.
String or UUID changes made by this patch:
Attachment #714937 - Attachment is obsolete: true
Attachment #716679 - Flags: review+
Attachment #716679 - Flags: approval-gaia-v1?(lsblakk)
Comment on attachment 716679 [details]
link to patch on github

minor fix, with significant win - approving for uplift to v1-train.
Attachment #716679 - Flags: approval-gaia-v1?(lsblakk) → approval-gaia-v1+
v1-train@001a24497c00379eacf2f257579b3df476baa245
It turns out that we also needed this patch on v1.0.1, per bug 854168, which was tef+, so I landed it there:

https://github.com/mozilla-b2g/gaia/commit/f8a2e1df9e99edbbef311aa661e6bd3fd697897e
Blocks: 854168
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: