889735 - [MMS] when attaching so many images,size of the attached MMS content is not refreshed even after removing some images

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title: when attaching so many images,size of the attached MMS content is not refreshed even after removing some images 
2. Precondition: Should be able to send and receive MMS
3. Tester's Action: 1) Add some images in order to show the information about maximum size reached pop-up
                    2) Remove some pics in order to meet the allowed size and try to send the MMS
4. Detailed Symptom (ENG.) : User is not able to send MMS after reducing its size (the pop-up is still shown even after removing the image). Only adding new pic refreshes the size and activates 'Send' button
5. Expected: The size of the message should be refreshed also after removing pics in order to allow sending it.
6. Reproducibility: Y
1) Frequency Rate : 100%
7. Gaia Master/v1-train: Reproduced on v1-train
8. Gaia Revision:  f2d6ed54a236e6e3b94f0abad9f0dacb8a1cc7b3
9. Personal email id: [e-mail]

Comment 1

[Leo]

This issue is happening because when we remove an image the state.size is not reset, and in wherever we use Compose.size we are getting the old size value.

But when we press back space state.size is reset, and value is recalculated next time compose.size is accessed.

If allowed, we can steal this issue and provide a patch for review :-)

Comment 2

[Julien Wajsberg [:julienw]]

I just asked kaze, he's ok, then please do !

Comment 3

[Leo]

Attachment: Pointer to pull request

Stealing from kaze

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 772026 [details]
Pointer to pull request

looks good to me, but could you please add a unit test to check that the size is recalculated in both cases (delete and update) ?

Comment 5

[Julien Wajsberg [:julienw]]

please ask review again when you update the pull request. thanks !

Comment 6

[Corey Frang [:gnarf]]

Attachment: patch v2

Took the fix from :Leo (Thanks!) and added some unit tests so we can land this.

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 773320 [details] [diff] [review]
patch v2

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

r=me with nits

::: apps/sms/test/unit/compose_test.js
@@ +381,5 @@
>          };
>          Compose.on('input', onInput);
>          Compose.append(mockImgAttachment());
> +        // we store this so we can make sure it gets resized
> +        actualSize = Compose.size;

makes probably more sense to store it before appending. So that if one day we change and calculate the size in "append" this won't break for nothing.

@@ +504,4 @@
>        Compose.append(this.attachment);
> +      this.sinon.stub(AttachmentMenu, 'open');
> +      this.sinon.stub(AttachmentMenu, 'close');
> +      this.size = Compose.size;

same here, makes more sense to store the size before append.

also, maybe call it "initialSize" instead of "size".

@@ +535,5 @@
>          });
>          test('closes the menu', function() {
>            assert.isTrue(AttachmentMenu.close.called);
>          });
> +        test('recaluclates size', function() {

nit: recalculates

Comment 8

[Corey Frang [:gnarf]]

Attachment: patch v3

carrying r=julienw - nits addressed

Comment 9

[Corey Frang [:gnarf]]

master: https://github.com/mozilla-b2g/gaia/commit/ad52bd865800903e54d4c765891c02005079a49f
v1-train: https://github.com/mozilla-b2g/gaia/commit/ff587d985531ad96430215aafd93dddc6de2739f

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8dbf69a17819

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: ff587d985531ad96430215aafd93dddc6de2739f