[RTL] [Camera] Preview swiping is not reversed

VERIFIED FIXED in 2.2 S10 (17apr)


4 years ago
4 years ago


(Reporter: tif, Assigned: dmarcos)


2.2 S10 (17apr)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master wontfix)



(4 attachments)

Posted file RTL preview.pdf
1. Switch phone to Arabic
2. Open Camera and take a few photos
3. Tap on icon to go to Preview.
4. Swipe right to go to new photo

Additional photos are found to the right and are reached by swiping left. The same interaction as in a non-RTL language.

Changing languages should flip the interaction and additional photos are found to the left. This correct behaviour is seen in Gallery.

Why should this block?
Gallery implements the correct behaviour. Camera preview and Gallery need to have matching interactions in order to avoid some massive user confusion.
I'm not sure which meta bug to attach this to - there isn't one for Camera.
blocking-b2g: --- → 2.2?
Flags: needinfo?(doliver)
(In reply to Tiffanie Shakespeare from comment #1)
> I'm not sure which meta bug to attach this to - there isn't one for Camera.

It's bug 1072002.
Blocks: camera-rtl
Flags: needinfo?(doliver)
RTL triage: P1 -- camera and gallery need to agree on directionality.
Priority: -- → P1
blocking-b2g: 2.2? → 2.2+

Comment 4

4 years ago
Diego or Punam: Who can take this?

Flags: needinfo?(pdahiya)
Flags: needinfo?(dmarcos)

Comment 5

4 years ago
Diego is taking this on, assigning to him.
Assignee: nobody → dmarcos
Flags: needinfo?(pdahiya)
Flags: needinfo?(dmarcos)

Comment 7

4 years ago
Attachment #8591926 - Flags: review?(jdarcangelo)

Comment 8

4 years ago
Submitted PR for 2.2. On master I would like to use Justin's carrousel that already implements RTL orientation. It's ready to land but it got postponed
Comment on attachment 8591926 [details] [review]
Pull Request on v2.2

One minor comment about simplifying the left/right swap in PreviewGalleryController.prototype.handleSwipe. If you agree, go ahead and change it, but I'm fine with it either way.
Attachment #8591926 - Flags: review?(jdarcangelo) → review+

Comment 10

4 years ago
Pushed review changes + unit tests.

Comment 11

4 years ago
Comment on attachment 8591925 [details] [review]
[gaia] dmarcos:bug1150706 > mozilla-b2g:v2.2

Carry on r+ from Justin
Attachment #8591925 - Flags: review+

Comment 12

4 years ago
Comment on attachment 8591925 [details] [review]
[gaia] dmarcos:bug1150706 > mozilla-b2g:v2.2

Who should be approving the PR?
Flags: needinfo?(hkoka)
Attachment #8591925 - Flags: feedback?
(In reply to Diego Marcos [:dmarcos] from comment #12)
> Who should be approving the PR?

Please set the approval‑gaia‑v2.2 flag on the PR and provide the info requested in the form.
Flags: needinfo?(hkoka)

Comment 14

4 years ago
Comment on attachment 8591925 [details] [review]
[gaia] dmarcos:bug1150706 > mozilla-b2g:v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Not RTL mode was implemented for the gallery preview carrousel
[User impact] if declined:
The carrousel in the gallery preview won't behave as expected in RTL mode
[Testing completed]:
Unit tests included
[Risk to taking this patch] (and alternatives if risky):
Low. The patch only implements small UI tweaks to reverse the swiping direction of the gallery preview on RTL mode.
[String changes made]:
Attachment #8591925 - Flags: feedback? → approval-gaia-v2.2?(doliver)
Attachment #8591925 - Flags: approval-gaia-v2.2?(doliver) → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/d50b8a3919a7b4d8d289f150d3b9bed704ebafa9
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S10 (17apr)

Comment 16

4 years ago
Posted video video.mp4
Hi Tiffanie,
I have verified this issue on latest build of Flame 2.2 with the same steps in comment 0. And the behaviour is same as that in Gallery. Could you help to confirm this issue is fixed or not? Thanks!
See attachment:video.MP4

Device: Flame 2.2 
Build ID               20150420002502
Gaia Revision          c15a2b6d3a783813959c2b3bffd2a131f4270b9e
Gaia Date              2015-04-17 17:49:32
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cc02ee38b252
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150420.041634
Firmware Date          Mon Apr 20 04:16:45 EDT 2015
Bootloader             L1TC000118D0
Flags: needinfo?(tshakespeare)


4 years ago
QA Whiteboard: [MGSEI-Triage+]
Checked out the v2.2 branch and it looks good! Thanks Sue!
Flags: needinfo?(tshakespeare)

Comment 18

4 years ago
Per comment 16 and comment 17, change status to VERIFIED,FIXED

Comment 19

4 years ago
Test case has been added in moztrap:
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.