Closed Bug 1062634 Opened 6 years ago Closed 5 years ago

[MMS] Portrait Images are misaligned when sending or receiving multiple attachments.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.0 affected, b2g-v2.1 affected, b2g-v2.2 affected)

RESOLVED FIXED
2.2 S3 (9jan)
Tracking Status
b2g-v2.0 --- affected
b2g-v2.1 --- affected
b2g-v2.2 --- affected

People

(Reporter: jthomas, Assigned: ythej, Mentored)

Details

(Whiteboard: [2.1-exploratory][lang=css][good first bug])

Attachments

(5 files)

Description:

Repro Steps:
1) Update a Flame to 20140903000204
2) Launch Messages App
3) Attach a portrait image and an audio or landscaped image.
4) Send MMS to another contact.

Actual: Attached portrait image is aligned to the right.

Expected: It is expected that the portrait image will be aligned to the left.

Flame 2.1

Environmental Variables:
Device: Flame 2.1 (319mb)
Build ID: 20140903000204
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: fb5e796da813
Version: 34.0a2 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Notes: Portrait images are also misaligned to the left when receiving multiple attachments.

Repro frequency: 100%
See attached: Logcat, Screenshot
Attached image PortraitMisaligned.png
This issue DOES occur on the Flame 2.2, Open_C 2.1(512mb), Flame 2.0 Open_C 2.0

Description: User attached portrait image is aligned to the right instead of the left.

Flame 2.2

Environmental Variables:
Device: Flame 2.2 Master (319mb)
BuildID: 20140903040203
Gaia: 52670853c17fc0d3d33065c667c0ce124c93b98f
Gecko: e58842c764dd
Version: 35.0a1 (2.2 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables:
Device: Flame 2.1 (512mb)
Build ID: 20140903000204
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: fb5e796da813
Version: 34.0a2 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Open C 2.1

Environmental Variables:
Device: Open_C 2.1
Build ID: 20140903000204
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: fb5e796da813
Version: 34.0a2 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0

Environmental Variables:
Device: Flame 2.0
BuildID: 20140903000206
Gaia: 449d8db9b3ea1f9262db822c37ef2143477172b7
Gecko: 13b41e22c8f6
Version: 32.0 (2.0)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Open_C 2.0

Environmental Variables:
Device: Open_C 2.0
Build ID: 20140903000206
Gaia: 449d8db9b3ea1f9262db822c37ef2143477172b7
Gecko: 13b41e22c8f6
Version: 32.0 (2.0)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [2.1-exploratory]
Flame 2.1 is not master which you stated twice.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(jthomas)
Looks like we have this type of alignment for ages, at least I see the same in v1.3. I tried to find spec on this, but only found examples in [1] where attachments are aligned to the left, like expected in comment 0.

NI Jenny, our UX expert, to confirm what alignment should be applied to the attachments.

Thanks!

[1] https://mozilla.app.box.com/applications/1/864518456/7994808358/1
Flags: needinfo?(jelee)
Please disregard my reference to the Master for the Flame 2.1 (512mb) and and Open_C 2.1. Variables have been updated.

Flame 2.1(512mb)

Environmental Variables:
Device: Flame 2.1 (512mb)
Build ID: 20140903000204
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: fb5e796da813
Version: 34.0a2
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Open C 2.1

Environmental Variables:
Device: Open_C 2.1
Build ID: 20140903000204
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: fb5e796da813
Version: 34.0a2
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage?]
Flags: needinfo?(jthomas) → needinfo?(ktucker)
Minor issue here so not nominating to block on this.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
(In reply to Oleg Zasypkin [:azasypkin] from comment #4)
> Looks like we have this type of alignment for ages, at least I see the same
> in v1.3. I tried to find spec on this, but only found examples in [1] where
> attachments are aligned to the left, like expected in comment 0.
> 
> NI Jenny, our UX expert, to confirm what alignment should be applied to the
> attachments.
> 

Hi there :) According to the latest visual spec, all media should be left aligned and the thumbnail will be square shape. See Bug 983172 attachment 8475043 [details] for details, thanks!
Flags: needinfo?(jelee)
Non-blocker, so removing this from blocking the user story bug.
No longer blocks: 1026690
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Mentor: felash
Whiteboard: [2.1-exploratory] → [2.1-exploratory][lang=css]
After 983172 landed, thumbnails for portrait and landscape images will be square so will be correctly aligned. However we want them to be left-aligned in the bubble, instead of right aligned.
Whiteboard: [2.1-exploratory][lang=css] → [2.1-exploratory][lang=css][good first bug]
Assignee: nobody → vish.thej
Pull request:
https://github.com/mozilla-b2g/gaia/pull/26889
Attachment #8538553 - Flags: review?(felash)
Mentor: felash → schung
Comment on attachment 8538553 [details] [diff] [review]
bug_1062634.patch

Redirecting to Steve!
Attachment #8538553 - Flags: review?(felash) → review?(schung)
Keywords: checkin-needed
after getting r+ only , then only put checkin-needed keyword.
(In reply to kumar rishav (:rishav_) from comment #12)
> after getting r+ only , then only put checkin-needed keyword.

Okay! :)
Comment on attachment 8538553 [details] [diff] [review]
bug_1062634.patch

Review of attachment 8538553 [details] [diff] [review]:
-----------------------------------------------------------------

I think you could simply remove the outgoing selector, keeping margin-right: auto isn't that necessary.

::: apps/sms/style/attachment.css
@@ +37,3 @@
>  }
>  
>  .article-list[data-type="list"] .message.outgoing .attachment-container {

Since all the attachments should align left by default, you can simply remove this selector and change the margin in general container selector ".article-list[data-type="list"] .message .attachment-container".
Attachment #8538553 - Flags: review?(schung)
Hi Steve Chung,

Thanks for the review. :)
Hi Steve,
I see that the attachment's are already aligned left in bubble when we remove the selector 
-.article-list[data-type="list"] .message.outgoing .attachment-container 

pull request:
https://github.com/mozilla-b2g/gaia/pull/26942
Attachment #8540058 - Flags: review?(schung)
Hi Vishnu, the patch is fine, but could you please rebase your commits into one and adding the "r=REVIEWER_NAME" at the end for merging? You can check out the git rebase command to squash the commits, thanks.
Flags: needinfo?(yvtheja)
Hi Steve, 
sorry that my repo got messed up yesterday.
Thanks. :)
Flags: needinfo?(yvtheja)
Attachment #8540865 - Flags: review?(schung)
Hi Vishnu, I just land another patch that might conflict with your patch, but i think the solution is still the same: Just remove the outgoing styling. Sorry for the inconvenience :(
Flags: needinfo?(yvtheja)
Comment on attachment 8540058 [details] [diff] [review]
bug_1062634.patch

Review of attachment 8540058 [details] [diff] [review]:
-----------------------------------------------------------------

Instead od creating another pull request, you can force push your new commit to the original branch
Attachment #8540058 - Flags: review?(schung)
Hi Steve,
Fetched again and rebased. :)
Thanks :)
Flags: needinfo?(yvtheja)
(In reply to Vishnu Teja from comment #18)
> Created attachment 8540865 [details] [review]
> pull request for the bug fix 1062634
> 
> Hi Steve, 
> sorry that my repo got messed up yesterday.
> Thanks. :)
Comment on attachment 8540865 [details] [review]
pull request for the bug fix 1062634

Looks good, thanks!
Attachment #8540865 - Flags: review?(schung) → review+
In master: 6f2b5a28da17cb75a1802958a2c1dda225898bb8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S3 (9jan)
You need to log in before you can comment on or make changes to this bug.