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

Status: RESOLVED FIXED

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

Description

[John Thomas [:Johnt]]

Attachment: logcat_20140903_1657.txt

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

Comment 1

[John Thomas [:Johnt]]

Attachment: PortraitMisaligned.png

Comment 2

[John Thomas [:Johnt]]

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

Comment 3

[KTucker [:KTucker][Inactive 3/4/2016]]

Flame 2.1 is not master which you stated twice.

Comment 4

[Oleg Zasypkin [:azasypkin]]

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

Comment 5

[John Thomas [:Johnt]]

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

Comment 6

[KTucker [:KTucker][Inactive 3/4/2016]]

Minor issue here so not nominating to block on this.

Comment 7

[Jenny Lee]

(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!

Comment 8

[Jason Smith [:jsmith]]

Non-blocker, so removing this from blocking the user story bug.

Comment 9

[Julien Wajsberg [:julienw]]

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.

Comment 10

[Vishnu Teja [:ythej]]

Attachment: bug_1062634.patch

Pull request:
https://github.com/mozilla-b2g/gaia/pull/26889

Comment 11

[Julien Wajsberg [:julienw]]

Comment on attachment 8538553 [details] [diff] [review]
bug_1062634.patch

Redirecting to Steve!

Comment 12

[kumar rishav (:rishav_)]

after getting r+ only , then only put checkin-needed keyword.

Comment 13

[Vishnu Teja [:ythej]]

(In reply to kumar rishav (:rishav_) from comment #12)
> after getting r+ only , then only put checkin-needed keyword.

Okay! :)

Comment 14

[Steve Chung [:steveck]]

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".

Comment 15

[Vishnu Teja [:ythej]]

Hi Steve Chung,

Thanks for the review. :)

Comment 16

[Vishnu Teja [:ythej]]

Attachment: bug_1062634.patch

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

Comment 17

[Steve Chung [:steveck]]

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.

Comment 18

[Vishnu Teja [:ythej]]

Attachment: pull request for the bug fix 1062634

Hi Steve, 
sorry that my repo got messed up yesterday.
Thanks. :)

Comment 19

[Steve Chung [:steveck]]

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 :(

Comment 20

[Steve Chung [:steveck]]

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

Comment 21

[Vishnu Teja [:ythej]]

Hi Steve,
Fetched again and rebased. :)
Thanks :)

Comment 22

[Vishnu Teja [:ythej]]

(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 23

[Steve Chung [:steveck]]

Comment on attachment 8540865 [details] [review]
pull request for the bug fix 1062634

Looks good, thanks!

Comment 24

[Steve Chung [:steveck]]

In master: 6f2b5a28da17cb75a1802958a2c1dda225898bb8