863885 - [Gallery] Far left border effect does nothing to the image

Status: RESOLVED FIXED

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

Description

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

## Environment :
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/f671fa539473
Gaia   e7e338a765e22334b40ced41489a785941382c66
BuildID 20130411070205
Version 18.0
Leo
## Repro :
1. launch gallery
2. select an image
3. select edit
4. select border
5. Notice that the highlight is on the far left border which shows a white border in the thumbnail, no border on the image preview
6. select checkmark

## Expected :
Border should be on the image

## Actual :
No Border on the final image, a duplicate image was made.

## Note :
Happens on Leo and Unagi

Comment 1

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

happens on 1.0.1 as well.

Occasionally the app will close. Not sure why.  Probably OOM?

Comment 2

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

I don't think the far left one is for creating a border for this image. It should be a "no" border option. It is for reseting. So, how do you think about the following expected and actual:

## Expected :
No border on the far left item.

## Actual :
A tiny border on the far left item.


It's a easy fix. But I need to confirm my thought is right on it.

Comment 3

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

Oh!  It was for resetting?

I think it would make sense with no borders.

Yes, I agree:
Expected : no border on far left image

Comment 4

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

Attachment: The patch of this bug

Change the border in css of the left most border effect to zero

Comment 5

[David Flanagan [:djf]]

Comment on attachment 753083 [details]
The patch of this bug

What an embarrassing bug! I would have sworn that it was a recent regression, but I looked and it goes back to my original code. I never noticed it, and I guess no one else did either.

Thank you for reporting and fixing this bug!

r=djf for github commit 5f7b7fb. But please don't land that commit. Instead, simplify the CSS as suggested on github and land a new version.

Comment 6

[David Flanagan [:djf]]

Comment on attachment 753083 [details]
The patch of this bug

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 #): not a regression; just a stupid bug no one noticed before.
User impact if declined: users will be confused by the border options in gallery edit mode 
Testing completed: yes
Risk to taking this patch (and alternatives if risky): no risk: trivial one-line CSS patch that just modifies a border style for one element.
String or UUID changes made by this patch: none

This seems like a no-brainer to uplift to v1-train.  It would be nice on v1.0.1 too, but I don't know how to request that.

Comment 7

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

merge to master:
https://github.com/mozilla-b2g/gaia/commit/b9b9d23e148fbfd7289adb83c09f1a0d107684d4


lsblakk, we have no idea how to request uplift, would you mind to help us and tell us how to do it after??

Comment 8

[David Flanagan [:djf]]

John, I think that what I did in comment 6 is enough to request uplift to v1-train.  If you also want uplift to v1.0.1 be sure that Lukas knows that's what you're asking for.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 753083 [details]
The patch of this bug

OK for uplift to v1.1 branch. At this stage uplifts to v1.0.1 have to be tef:+ which has to be approved by partners and the ship has sailed for cosmetic fixes such as this.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

John, can you please uplift this?

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

Hey,  I tried to uplift this commit onto v1-train but there was a small merge conflict in gallery.css:

++<<<<<<< HEAD
 +  left: calc(0% + 3px);
 +  border: 2px solid white;
++=======
+   left: calc(0% + 0.3rem);
+   border: 0;
++>>>>>>> b9b9d23... Merge pull request #9943 from huchengtw-moz/gallery/Bug_863885_far_left_border_effect_does_nothing

Comment 12

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

John Ford,

The later one is correct: 

   left: calc(0% + 0.3rem);
   border: 0;

And, sorry, I am new to here. What should I do for this patch?? make another PR for v1-train or what?

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to John Hu [:johnhu] from comment #12)
> John Ford,
> 
> The later one is correct: 
> 
>    left: calc(0% + 0.3rem);
>    border: 0;
> 
> And, sorry, I am new to here. What should I do for this patch?? make another
> PR for v1-train or what?

No need for a pull request, just create a branch in your repository that is based on v1-train and has a full conflict resolution of this cherry-pick.  Assuming your mozilla-b2g remote is called 'origin' and your personal remote is called 'johnhu', you might be able to do:

cd gaia
git checkout -t origin/v1-train -b bug863885-v1-train
git cherry-pick -x -m1 b9b9d23e148fbfd7289adb83c09f1a0d107684d4
<resolve conflicts>
git add apps/gallery/style/gallery.css
git commit -a
git push johnhu bug863885-v1-train

And I can fetch that commit and cherry-pick it onto v1-train.

Comment 14

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

John, 

This is the url of my branch based on v1-train.

https://github.com/huchengtw-moz/gaia/tree/bug-863885-v1-train

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: 89d0d9ba3ecaea543859be28ee0ccda13184f506