[Gallery] - Update image edit crop flow

RESOLVED FIXED

Status

Firefox OS
Gaia::Gallery
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pdahiya, Assigned: pdahiya)

Tracking

({polish})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: interaction-design)

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

3 years ago
This is a follow up of bug 1042319. Scope of this bug includes
a) Update edit crop tool as per 2.2 UX spec - https://github.com/mozilla-b2g/gaia-specs/blob/master/%5B2.2%5D%20Gallery%20Edit.pdf

b) Update edit crop tool as per visual specs attached to meta bug 1024258
https://bugzilla.mozilla.org/show_bug.cgi?id=1024258#c7
(Assignee)

Updated

3 years ago
Assignee: nobody → pdahiya
(Assignee)

Comment 1

3 years ago
Created attachment 8517712 [details] [review]
PR for image editor crop tool updates

Attaching WIP patch for updating crop flow as per news specs, patch is updated with
1. Crop handles appear inside edges of image to avoid edge swipe gesture between dragging edges of crop tool and side gesture for jumping to another app.
2. Remove “undo/back” button
3. Retain original image and crop settings within Edit session when user returns to crop feature.

Functionality TBD
1. Introduce additional aspect ratios 
2. Visual updates - New icons for crop aspect ratio and highlight state
3. Tests

David, Katie, Amy
I am sharing WIP crop changes to get feedback on the implemented flow and get sense of minimum work left to get updated crop tool on master before switching gear to other gallery priorities. Thanks!
Attachment #8517712 - Flags: ui-review?(kcaldwell)
Attachment #8517712 - Flags: ui-review?(amlee)
Attachment #8517712 - Flags: feedback?(dflanagan)

Comment 2

3 years ago
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Hi Punam,

Here’s my feedback:

1. Can you use the .svgs of the handles that I had provided in the metabug? These are still currently using the old ones. 

2. The handles should extend outside of the frame by 1px (see visual spec)

3. The handles should have a 100% opacity. It currently looks like it’s semi-transparent. (See spec).

4. The crop area background colour should be #333333 (Dark grey) and no black. This should be part of the common controls. 

Thanks!
Attachment #8517712 - Flags: ui-review?(amlee) → ui-review-

Comment 3

3 years ago
Punam, 
Cropping is looking great and matches the spec flow. Nice job! 

I had one unexpected experience...
STR:
1. open gallery » select an image
2. tap edit option » select crop tool » crop image
3. let phone go to sleep

Result:
When phone wakes up, Gallery is still in edit-crop mode, with remembered crop, but image is missing (?) - all black

Expected:
When phone wakes up, Gallery is still in edit-crop mode, with remembered crop, with image visible
I haven't looked at the code yet, but just trying out the patch, I have these comments:

1) The new flow seems to be working right

2) I like the crop handles on the inside, though I see that Amy wants them at least 1px outside. I suspect that the highlight circle we draw when the user touches a handle would be better as a semi or quarter circle so that it never goes outside of the rectangle. 

3) I think we might need to recompute and reapply the auto correct (magic wand) when the crop region changes.  If I crop in tightly on a small part of the image, then apply the magic wand, then go back and undo the crop, I can end up with an image that looks over corrected.  (I suspect this is an existing bug, though and not something new introduced here.)
(In reply to katieC from comment #3)
> Punam, 
> Cropping is looking great and matches the spec flow. Nice job! 
> 
> I had one unexpected experience...
> STR:
> 1. open gallery » select an image
> 2. tap edit option » select crop tool » crop image
> 3. let phone go to sleep
> 
> Result:
> When phone wakes up, Gallery is still in edit-crop mode, with remembered
> crop, but image is missing (?) - all black
> 
> Expected:
> When phone wakes up, Gallery is still in edit-crop mode, with remembered
> crop, with image visible

I suspect that this is an unrelated gecko bug and that it probably occurs even without Punam's patch. It might be a recent regression on master since I've seen some wonky graphics stuff recently. Or it could date back to the webgl changes (for the contextlost event) of a few months back. Punam: if this reproduces even without your patch, please file a bug (and needinfo me on it)
(In reply to Amy Lee [:amylee] from comment #2)
> Comment on attachment 8517712 [details] [review]
> WIP PR for crop tool updates
> 
> Hi Punam,
> 
> Here’s my feedback:
> 
> 1. Can you use the .svgs of the handles that I had provided in the metabug?
> These are still currently using the old ones. 
> 

The app actually draws those crop handles in a <canvas> tag, so we can't use svgs for that.  If we were using CSS to draw the handles, I think we could use your specs in the doc. As it is, using canvas, I think that Punam can get close to what you want but may not be able to match it precisely without significant extra work.

Since we're not doing a full visual refresh for 2.2 and are de-prioritizing these edit changes, I'd suggest that Punam just take whatever easy steps she can to get the handles looking right. We can come back to it and get them pixel perfect later when we (for example) add the grid lines that are now part of the crop spec.

Punam: the tricky part about the handles that Amy has specified is the asymmetrical curvature on the inside and outside. Currently I think we just draw lines, and I don't think you can get exactly what Amy wants with just lines--you'd have to define a full path and fill it to be precise. But I think you can get quite close if you define a cropping rectangle that is one pixel outside of the crop border and then draw extra wide lines that are on that crop rectangle. The part of the line (and the part of the endcap) that is inside the crop rectangle will draw, and the part that is outside will be cut off cleanly. That same crop region will probably improve the appearance of the highlight circles displayed when the user drags the handles (though I note that those are not in Amy's spec at all). Again though, the key here is to just get something that is good enough for 2.2 so you can move on to higher priority work.

> 2. The handles should extend outside of the frame by 1px (see visual spec)

That should be easy.

> 3. The handles should have a 100% opacity. It currently looks like it’s
> semi-transparent. (See spec).

That should be easy.
 
> 4. The crop area background colour should be #333333 (Dark grey) and no
> black. This should be part of the common controls. 

Easy, but I suggest that we defer this until we also change the background color when viewing the images. Right now it looks like we've got the same black background in both cases.
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Feedback+: I think you're on the right track here. But see my comments on github. I think if you take the refactoring a little further you can end up with cleaner code. Right now it feels like you're tacking this change on to code that is designed to do cropping the old way instead of just changing the code thoroughly to do cropping the new way.
Attachment #8517712 - Flags: feedback?(dflanagan) → feedback+
(Assignee)

Comment 8

3 years ago
(In reply to David Flanagan [:djf] from comment #5)
> (In reply to katieC from comment #3)
> > Punam, 
> > Cropping is looking great and matches the spec flow. Nice job! 
> > 
> > I had one unexpected experience...
> > STR:
> > 1. open gallery » select an image
> > 2. tap edit option » select crop tool » crop image
> > 3. let phone go to sleep
> > 
> > Result:
> > When phone wakes up, Gallery is still in edit-crop mode, with remembered
> > crop, but image is missing (?) - all black
> > 
> > Expected:
> > When phone wakes up, Gallery is still in edit-crop mode, with remembered
> > crop, with image visible
> 
> I suspect that this is an unrelated gecko bug and that it probably occurs
> even without Punam's patch. It might be a recent regression on master since
> I've seen some wonky graphics stuff recently. Or it could date back to the
> webgl changes (for the contextlost event) of a few months back. Punam: if
> this reproduces even without your patch, please file a bug (and needinfo me
> on it)
I am able to replicate the issue without the patch and has created bug 1098541 to debug and fix the issue. Thanks

Comment 9

3 years ago
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

changing ui review to Tif (I've walked her through the Gallery-Edit spec)
Attachment #8517712 - Flags: ui-review?(kcaldwell) → ui-review?(tshakespeare)
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Two things I noticed:

1) When using the corner handles, I noticed that when one side snaps to the side the corner won't fully move in all directions. The handle should always resize the crop area in any direction.

2)I noticed that if I was in edit mode and my screen turned off, when unlocking the phone the image disappeared.

Additionally, it looks like the crop handles need to be moved slightly so that they straddle the crop area. This is important b/c on light background, the handles are getting lost in the initial view.
Attachment #8517712 - Flags: ui-review?(tshakespeare) → ui-review-
Whiteboard: interaction-design
Keywords: polish
(Assignee)

Comment 11

3 years ago
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Hi David

Please provide feedback on the attached patch updated with additional aspect ratios 4:3, 3:4, 16:9. 

Also, I did the search and Bug 1116378 references the UX issue around undo button in crop mode. Thanks!
Attachment #8517712 - Flags: feedback+ → feedback?(dflanagan)
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

This looks good. I've left a few suggestions on github but overall it seems fine to me.  (Note, though that I did not try running the code).

Note also that now that cropping is always done on the full image, the cropImage() method is more complicated than it needs to be. I think you now call resetCropRegion() before ever showing the crop overlay, so this.source is reset to the full image size. So this comment from cropImage() is out of date:

```
  // Note that the original image may have already been cropped, so we
  // multiply by the size of the crop region, not the full size
```

And also, the += in these two lines can just be changed to =, I think:

```
  this.source.x += left;
  this.source.y += top;
```

Finally, if you're working on cropImage(), you could tackle this:

```
  // XXX: tweak these to make sure we still have the right aspect ratio
  // after rounding to pixels
  console.error('Maintain aspect ratio precisely!!!');
```

I punted on this long ago. The issue here is that for a ratio like 3:4, this does does not enforce that `this.source.width/this.source.height === 3/4`.

What I'd like to do here is something like this:

- if there is no aspect ratio do nothing
- if the aspect ratio is 1:1, then make sure width and height are the same, or make the smaller one bigger to match the bigger one. (They shouldn't be off by more than 1 pixel, I think)
- if there was some other crop aspect ratio x:y, then pick the smaller of x or y.
- if x is smaller, then adjust this.source.width up or down to the nearest non-zero multiple of x. And then set this.source.height to y*this.source.width/x
- if y is smaller than x, then adjust the height to the nearest multiple of y and set the width based on that.

(Actually, maybe a better algorithm would be to see how close the width is to a multiple of x and the height is to a multiple of y and adjust whichever one requires the smaller adjustment. Then set the other dimension based on that.)

You don't have to make this change now. We can do it later as a separate bug.
Attachment #8517712 - Flags: feedback?(dflanagan) → feedback+
(Assignee)

Comment 13

3 years ago
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Hi David
I have updated the PR with your feedback. As new icon updates are touching shared files, I will submit those changes as a separate pull requests. Please review. Thanks
Attachment #8517712 - Attachment description: WIP PR for crop tool updates → PR for image editor crop tool updates
Attachment #8517712 - Flags: review?(dflanagan)
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

The code looks good. I've tried it out, including the cropping for a pick activity, and it works well.
Attachment #8517712 - Flags: review?(dflanagan) → review+
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Will try to review today/tomorrow.
Attachment #8517712 - Flags: ui-review?
Attachment #8517712 - Flags: ui-review-
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

sorry - idk why it removed our names
Attachment #8517712 - Flags: ui-review?(tshakespeare)
Attachment #8517712 - Flags: ui-review?(amlee)
Attachment #8517712 - Flags: ui-review?
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

I'm still seeing the issue from my previous comment:

When using the corner handles, I noticed that when one side snaps to the side the corner won't fully move in all directions. The handle should always resize the crop area in any direction.

I've also been able to get it to crash about 75% of the time I play with cropping :(

When I am able to apply the crop and save a copy, about half the time I'm shown the wrong image when I'm back in gallery instead of the new copy. It looks like it's just showing me the most recent item in gallery (which isn't the copy - if that makes sense). I'm not totally sure why or what triggers it.

Thanks for working on this!
Attachment #8517712 - Flags: ui-review?(tshakespeare) → ui-review-
(In reply to Tiffanie Shakespeare from comment #16)
> Comment on attachment 8517712 [details] [review]
> PR for image editor crop tool updates
> 
> sorry - idk why it removed our names

Hi, 

I'm having issues with flashing patches to my phone. Hopefully IT can help me out tomorrow morning and I will reply with my feedback. Thanks
(Assignee)

Comment 19

3 years ago
(In reply to Tiffanie Shakespeare from comment #17)
> Comment on attachment 8517712 [details] [review]
> PR for image editor crop tool updates
> 
> I'm still seeing the issue from my previous comment:
> 
> When using the corner handles, I noticed that when one side snaps to the
> side the corner won't fully move in all directions. The handle should always
> resize the crop area in any direction.

Sorry for not replying to this comment earlier. Attached patch should not affect moving of crop handles as that code is untouched. Please confirm if you see above experience in master without patch.
> 
> I've also been able to get it to crash about 75% of the time I play with
> cropping :(
> 
Sorry its pain, the crashes seen are unrelated and due to bug 1119157 that should be fixed soon. You can confirm it by seeing in logs error 'InternalError: too much recursion'

> When I am able to apply the crop and save a copy, about half the time I'm
> shown the wrong image when I'm back in gallery instead of the new copy. It
> looks like it's just showing me the most recent item in gallery (which isn't
> the copy - if that makes sense). I'm not totally sure why or what triggers
> it.
> 

This issue should never happen in a production build as the edited copy will be saved with most recent date and time and should show up first in thumbnail list.

I have seen it when I flash my phone with old build id which sets date and time on my phone to earlier date. Easy way to work around this issue is use latest build (say March 17) and delete any photos from gallery app with date March 17th. 


> Thanks for working on this!
(Assignee)

Updated

3 years ago
Flags: needinfo?(tshakespeare)
(Assignee)

Comment 20

3 years ago
(In reply to Amy Lee [:amylee] from comment #2)
> Comment on attachment 8517712 [details] [review]
> PR for image editor crop tool updates
> 
> Hi Punam,
> 
> Here’s my feedback:
> 
> 1. Can you use the .svgs of the handles that I had provided in the metabug?
> These are still currently using the old ones. 
>
As explained by David in comment #6 , crop handles are drawn in canvas and it's a big change to use svg as handles. Attached patch tries to come closer to specs by updating handles to have square line cap instead of rounded. This gives outside and inside corner of crop handle 0 rem radius.

> 2. The handles should extend outside of the frame by 1px (see visual spec)
> 
Done

> 3. The handles should have a 100% opacity. It currently looks like it’s
> semi-transparent. (See spec).
> 

Done

> 4. The crop area background colour should be #333333 (Dark grey) and no
> black. This should be part of the common controls. 
> 

This update is deferred for now as explained in comment #6.

> Thanks!
Created attachment 8579399 [details]
Gallery_Crop_Edits.png

Hi 

Here is my feedback:

1. Can you make sure that the white outline frame is flushed to the edge of the image? Only the handles should be offset by 1px outside the frame. 

2. The handle graphics looks fuzzy. Also if we can’t use the .svg files then I would go back to rounded corners

3. The dark overlay on the photo seems to be offset by 1px.

4. I also noticed that in gallery, the status bar is missing. Is this correct? (NI Tiff).

Thanks!

Updated

3 years ago
Attachment #8517712 - Flags: ui-review?(amlee) → ui-review-
(Assignee)

Comment 22

3 years ago
(In reply to Amy Lee [:amylee] from comment #21)
> Created attachment 8579399 [details]
> Gallery_Crop_Edits.png
> 
> Hi 
> 
> Here is my feedback:
> 
> 1. Can you make sure that the white outline frame is flushed to the edge of
> the image? Only the handles should be offset by 1px outside the frame. 

This seems hard to replicate, attaching screen shot where the border looks flushed. I might be missing something here, including David to provide his input.

> 
> 2. The handle graphics looks fuzzy. Also if we can’t use the .svg files then
> I would go back to rounded corners
> 
Updated patch to use round corners
> 3. The dark overlay on the photo seems to be offset by 1px.
> 

Updated patch to extend translucent gray area by 1px. David, is it safe to increase gray area by 1px in both dimensions.  https://github.com/mozilla-b2g/gaia/pull/25854/files#diff-6e48fa715fc315697388e7075d015a41R1232


> 4. I also noticed that in gallery, the status bar is missing. Is this
> correct? (NI Tiff).
> 
> Thanks!
Flags: needinfo?(dflanagan)
(Assignee)

Comment 23

3 years ago
Created attachment 8579538 [details]
screen shot crop overlay -03-18
I'd really like to treat this bug (as its title suggests) as a UX bug to update the flow, and leave all the visual issues for another bug. If we wait to fix all the visual nits in this bug it will be weeks before we can land it.

Here are my comments on the particular issues that Tif and Amy have raised:

(In reply to Tiffanie Shakespeare from comment #17)

> When using the corner handles, I noticed that when one side snaps to the
> side the corner won't fully move in all directions. The handle should always
> resize the crop area in any direction.

That's weird. But it reproduces easily on master, which probably means it is my fault from long ago. Let's file a new bug for fixing this. Thanks for finding it, Tif.
 
> I've also been able to get it to crash about 75% of the time I play with
> cropping :(

As Punam explained that was a Gonk bug affecting all apps that use workers. Fixed now.

> When I am able to apply the crop and save a copy, about half the time I'm
> shown the wrong image when I'm back in gallery instead of the new copy. It
> looks like it's just showing me the most recent item in gallery (which isn't
> the copy - if that makes sense). I'm not totally sure why or what triggers
> it.

Punam's explanation of this makes sense to me. If when flashing your phone you end up with the wrong time and there are photos that the app things were taken in the future, then those will show first.

In any case, this bug is unrelated to the current patch, so if there is a bug here, let's file and investigate it separately.

(In reply to Amy Lee [:amylee] from comment #21)
> 
> 1. Can you make sure that the white outline frame is flushed to the edge of
> the image? Only the handles should be offset by 1px outside the frame. 

I don't understand this. The frame looks flush to me. Are you saying that the frame extends too far outside of the image, or that it does not get close enough to the edge?  This might be related to the fuzziness...
 
> 2. The handle graphics looks fuzzy. 

That's probably a CSS vs device pixel issue. The canvas used for the overlay is perhaps just being sized in CSS pixels rather than in device pixels. So it might be that we're drawing handles that are 5px wide and then resizing them to be 7.5 device pixels when displayed.  

Before bug 1102712 were were displaying the preview image itself at low resolution and ignoring the devicePixelRatio. I fixed that back in December.  But I left the overlay that displays the crop handles using CSS pixels instead of device pixels.  I think maybe I need to increase set the size of that canvas in device pixels and set a scale on it so that we can draw on it using CSS pixels. That would probably solve the fuzzy issue.

Again, though, I think this is a long-standing bug or maybe a regression from 1102712, but not related to this bug.

> Also if we can’t use the .svg files then
> I would go back to rounded corners
 
Eventually we can draw the handles exactly how you want them. But I told Punam that was out of scope for this bug. Using square corners was an attempt to easily get closer to your spec.  It looks like Punam has already reverted to rounded, however.

> 3. The dark overlay on the photo seems to be offset by 1px.

It is on master, too. I think that is a regression I caused with bug 1102712. Thanks for pointing it out. Let's file a separate bug for it.

> 4. I also noticed that in gallery, the status bar is missing. Is this
> correct? (NI Tiff).

If there is an issue with that, its a system app bug, not a gallery bug.

> Thanks!

Punam: you asked whether we can just increase the gray area by 1px. Given that this bug exists on master, I'd say don't try to fix it here. Let's file a separate bug and fix that along with the fuzzy handles. 

Here's what I suggest:

- ask Tif to review just the new crop UX and land this patch when she's satisfied with that.

- file a new bug (I think you already have) for the new icon font stuff

- file a new bug to make the overlay canvas honor devicePixelRatio which should fix the fuzzy handles and the mis-aligned gray rectangle.

- file a new bug to fix the crop handles that get stuck when against one edge.

- file a new bug to get the crop handles to look just they way Amy wants them.
Flags: needinfo?(dflanagan)
(Assignee)

Comment 25

3 years ago
(In reply to David Flanagan [:djf] from comment #24)

> Punam: you asked whether we can just increase the gray area by 1px. Given
> that this bug exists on master, I'd say don't try to fix it here. Let's file
> a separate bug and fix that along with the fuzzy handles. 
> 
> Here's what I suggest:
> 
> - ask Tif to review just the new crop UX and land this patch when she's
> satisfied with that.
>  
> - file a new bug (I think you already have) for the new icon font stuff
> 
It's worked on in bug 1064969

> - file a new bug to make the overlay canvas honor devicePixelRatio which
> should fix the fuzzy handles and the mis-aligned gray rectangle.
> 
Created bug 1145298

> - file a new bug to fix the crop handles that get stuck when against one
> edge.
> 
Created bug 1145301

> - file a new bug to get the crop handles to look just they way Amy wants
> them.
Created bug 1145305
(Assignee)

Comment 26

3 years ago
Hi Tif

Setting ui-review flag to review attached patch for below features as described in specs https://github.com/mozilla-b2g/gaia-specs/blob/master/%5B2.2%5D%20Gallery%20Edit.pdf

1. Remove “undo/back” button (as noted in bug 1116378)
2. Retain original image and crop settings within Edit session when user returns to crop feature.
3. Introduce additional aspect ratios.
4. Crop handles appear inside edges of image to avoid edge swipe gesture between dragging edges of crop tool and side gesture for jumping to another app.

Thanks!


The
(Assignee)

Updated

3 years ago
Attachment #8517712 - Flags: ui-review- → ui-review?(tshakespeare)

Comment 27

3 years ago
Created attachment 8580265 [details] [review]
[gaia] punamdahiya:Bug1087494 > mozilla-b2g:master
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

So sorry for the delay! Patch looks good and meets requirements as Punam said in comment 26. 

Also, Gallery should be a full screen app so no status bar is shown. 

Thanks for opening those extra bugs to cover the additional issues!
Flags: needinfo?(tshakespeare)
Attachment #8517712 - Flags: ui-review?(tshakespeare) → ui-review+
(Assignee)

Comment 29

3 years ago
Comment on attachment 8580265 [details] [review]
[gaia] punamdahiya:Bug1087494 > mozilla-b2g:master

Thanks Tif and David for review. Setting review flag on the link created by autolander. I will appreciate if you can move your r+ on the autolander link for the same PR.
Attachment #8580265 - Flags: ui-review?(tshakespeare)
Attachment #8580265 - Flags: review?(dflanagan)
(Assignee)

Comment 30

3 years ago
Comment on attachment 8517712 [details] [review]
PR for image editor crop tool updates

Marking the non-autolander link file obsolete
Attachment #8517712 - Attachment is obsolete: true
Comment on attachment 8580265 [details] [review]
[gaia] punamdahiya:Bug1087494 > mozilla-b2g:master

Re-adding ui-review + per Punam's request. See my comment 28 for previous +.
Attachment #8580265 - Flags: ui-review?(tshakespeare) → ui-review+

Updated

3 years ago
Attachment #8580265 - Flags: review?(dflanagan) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Comment 32

3 years ago
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/d8d7bf01595274f76abe7ac833fb622320116612

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
I had to revert this in https://github.com/mozilla-b2g/gaia/commit/4e53a69221b852c79825c9e4857d162c7b855ad5 for l10n failures on b2g-inbound:
https://treeherder.mozilla.org/logviewer.html#?job_id=1570800&repo=b2g-inbound
Status: RESOLVED → REOPENED
Flags: needinfo?(pdahiya)
Resolution: FIXED → ---
(Assignee)

Comment 34

3 years ago
(In reply to Wes Kocher (:KWierso) from comment #33)
> I had to revert this in
> https://github.com/mozilla-b2g/gaia/commit/
> 4e53a69221b852c79825c9e4857d162c7b855ad5 for l10n failures on b2g-inbound:
> https://treeherder.mozilla.org/logviewer.html#?job_id=1570800&repo=b2g-
> inbound

Hi Wes

This error was never caught in gaia-try, https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=4bd627eff6e7 and only seen on b2g-inbound. Is there any way to catch b2g-inbound errors before landing PR on master. Thanks
Flags: needinfo?(pdahiya) → needinfo?(wkocher)

Comment 35

3 years ago
Created attachment 8582666 [details] [review]
[gaia] punamdahiya:Bug_1087494 > mozilla-b2g:master
(Assignee)

Comment 36

3 years ago
Hi David

The previous PR (attachment 8580265 [details] [review]) got reverted because of duplicate string L10n error only seen on b2g-inbound. I have removed duplicate l10n strings (crop-aspect-3x4.ariaLabel, crop-aspect-4x3.ariaLabel, crop-aspect-16x9.ariaLabel) and created new PR. Please review. Thanks
(Assignee)

Updated

3 years ago
Attachment #8582666 - Flags: review?(dflanagan)

Updated

3 years ago
Attachment #8582666 - Flags: review?(dflanagan) → review+
(Assignee)

Updated

3 years ago
Attachment #8580265 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Comment 37

3 years ago
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/9ba16c09fa6934b06f900bcc564e98fea803b1dc

Updated

3 years ago
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
(In reply to Punam Dahiya from comment #34)
> (In reply to Wes Kocher (:KWierso) from comment #33)
> > I had to revert this in
> > https://github.com/mozilla-b2g/gaia/commit/
> > 4e53a69221b852c79825c9e4857d162c7b855ad5 for l10n failures on b2g-inbound:
> > https://treeherder.mozilla.org/logviewer.html#?job_id=1570800&repo=b2g-
> > inbound
> 
> Hi Wes
> 
> This error was never caught in gaia-try,
> https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=4bd627eff6e7
> and only seen on b2g-inbound. Is there any way to catch b2g-inbound errors
> before landing PR on master. Thanks

I'm unsure. The builds on b2g-inbound have a "B" symbol, while I don't see those at all on the gaia-try push.
Flags: needinfo?(wkocher)
You need to log in before you can comment on or make changes to this bug.