[RTL][Camera] When setting a preview photo to wallpaper, the photo does not fit the screen

VERIFIED FIXED in 2.2 S7 (6mar)

Status

defect
P2
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: psiphantong, Assigned: pdahiya)

Tracking

unspecified
2.2 S7 (6mar)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

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

Details

Attachments

(2 attachments)

Reporter

Description

5 years ago
Description:
When setting a preview photo to wallpaper, the photo does not fit the screen


Repro Steps:
1) Update a Flame to 20141223010202
2) Go to Camera > take a photo
3) Tap the preview > tap share button
4) Tap wallpaper


Actual:
photo does not fit the screen


Expected:
photo displays correctly 


Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141223010202
Gaia: c2da2bafd4e809317e2ca70c9bf5c11136a32818
Gecko: 0532f2509f3f
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0


Repro frequency:100%
See attached: screenshot
Reporter

Updated

5 years ago
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
Flags: needinfo?(dharris)
Reporter

Updated

5 years ago
Blocks: camera-rtl
Summary: [Camera] When setting a preview photo to wallpaper, the photo does not fit the screen → [RTL][Camera] When setting a preview photo to wallpaper, the photo does not fit the screen
Just checked, this is not occurring anymore.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WORKSFORME
This issue still seems to be occurring with 10/10 attpemts on todays build. Reopening this for now.

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141230010205
Gaia: 322ef5116a5827a30c9a3cd9b842449a9c66a5b3
Gecko: 67872ce17918
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → REOPENED
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(dharris)
Resolution: WORKSFORME → ---
Assignee: nobody → nefzaoui
This just works fine here,
No longer occurring.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → WORKSFORME

Updated

4 years ago
Flags: in-moztrap-
This issue is occurring again in both 2.2 and 3.0 following the STR from comment 0


Device: Flame 3.0 (KK - Nightly - Full Flash)
Build ID: 20150218010226
Gaia: 82f286f10a41aab84a0796c89fbefe67b179994b
Gecko: 9696d1c4b3ba
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0


Device: Flame 2.2 (KK - Nightly - Full Flash)
Build ID: 20150217002515
Gaia: ea64caf6d4ab03fc4472eca9f41f20d651d55fa9
Gecko: 78d823f7be4c
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
blocking-b2g: --- → 2.2+
Priority: -- → P2
Assignee

Comment 5

4 years ago
Issue reported is specific to wallpaper app share activity in RTL mode and can be replicated by
a) sharing preview image as wallpaper from camera app 
b) sharing image as wallpaper from gallery app

Updating component to wallpaper and taking bug to investigate and submit fix.
Assignee

Updated

4 years ago
Component: Gaia::Camera → Gaia::Wallpaper
Assignee

Updated

4 years ago
Assignee: nefzaoui → pdahiya
Assignee

Comment 6

4 years ago
Assigning myself. Ahmed, Please let me know if you are looking into this bug. Thanks!
Assignee

Comment 8

4 years ago
Comment on attachment 8570156 [details] [review]
[gaia] punamdahiya:Bug1115178 > mozilla-b2g:master

Hi David

Please review attached patch that fixes shift in image while sharing as wallpaper in RTL mode. 

I have tested and for wallpaper share activity allowing parent div preview container to have same size as previewImage div fixes this issue.

Another approach is to check for RTL mode and handle translate set on previewImage on L119 and L145  but prefer simpler css fix.

https://github.com/mozilla-b2g/gaia/blob/master/apps/wallpaper/js/share.js#L119

Thanks
Punam
Attachment #8570156 - Flags: review?(dflanagan)
Comment on attachment 8570156 [details] [review]
[gaia] punamdahiya:Bug1115178 > mozilla-b2g:master

r- because I don't understand why this patch works, and because it seems too brittle. Someone could easily add the width:100% back in not realizing that it would break rtl.

Here's how I'd fix it, and if this looks good to you, then you can assume you have my r+ to land this patch.  Instead of removing the width setting, leave that in, and add the following to the #preview styles:

  /*
   * The image positioning code in share.js assumes the preview image
   * is positioned in the upper-left corner
   */
  direction: ltr;

This forces the the image to be laid out ltr in its container so that the transforms we apply are correctly computed.  Since the image is the only child of the container, we don't have to worry about messing up the direction of anything else.
Attachment #8570156 - Flags: review?(dflanagan) → review-
Test case has been added in moztrap:

https://moztrap.mozilla.org/manage/case/15922/
Flags: in-moztrap- → in-moztrap+
Assignee

Comment 11

4 years ago
(In reply to David Flanagan [:djf] from comment #9)
> Comment on attachment 8570156 [details] [review]
> [gaia] punamdahiya:Bug1115178 > mozilla-b2g:master
> 
> r- because I don't understand why this patch works, and because it seems too
> brittle. Someone could easily add the width:100% back in not realizing that
> it would break rtl.
> 
> Here's how I'd fix it, and if this looks good to you, then you can assume
> you have my r+ to land this patch.  Instead of removing the width setting,
> leave that in, and add the following to the #preview styles:
> 
>   /*
>    * The image positioning code in share.js assumes the preview image
>    * is positioned in the upper-left corner
>    */
>   direction: ltr;
> 
> This forces the the image to be laid out ltr in its container so that the
> transforms we apply are correctly computed.  Since the image is the only
> child of the container, we don't have to worry about messing up the
> direction of anything else.

Agreed this is better fix , updated css with review feedback. I guess I need to set review flag for the benefit of autolander.
Assignee

Updated

4 years ago
Attachment #8570156 - Flags: review- → review?(dflanagan)
Attachment #8570156 - Flags: review?(dflanagan) → review+
Assignee

Updated

4 years ago
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Assignee

Comment 13

4 years ago
Comment on attachment 8570156 [details] [review]
[gaia] punamdahiya:Bug1115178 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):Not a regression
[User impact] if declined: In RTL mode from camera preview and gallery app, when an image is shared as wallpaper, the image will not fit the screen and display shifted.

[Testing completed]: On master
[Risk to taking this patch] (and alternatives if risky):very low
[String changes made]:none
Attachment #8570156 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8570156 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
This issue is verified fixed on Flame 3.0 and on Flame 2.2. While setting a wallpaper either via Camera Preview or via Gallery, the confirmation page now displays the wallpaper correctly in RTL (Arabic).

Device: Flame 3.0 Master (full flash 319MB)
BuildID: 20150304010324
Gaia: 3fc0ac309f5fb0c1fe82c12223b955a4efce27e6
Gecko: c5b90c003be8
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Device: Flame 2.2 (full flash 319MB)
BuildID: 20150304002529
Gaia: 8b4b3e4b7e7c308764f71542437fd60625ac6b75
Gecko: 2cb52b7cda5a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage?][rtl-impact]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.