[Flame][v1.4][MMS] Edit text messages, add video no size limit tips.

RESOLVED FIXED in 2.2 S3 (9jan)

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: greatwall2686, Assigned: rishav_, Mentored)

Tracking

unspecified
2.2 S3 (9jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sms-papercuts])

Attachments

(1 attachment)

Reporter

Description

5 years ago
* Description: Add video without tip size limit when add message attachments. (add music, pictures did not prompt)


* Reproduction steps:
  1. New a message
  2. Press new message icon
  3. Press add attachment icon
  5. From files choose a video
  6. Check result

* Expected result:
  When choose the file is too large should be a size limit. 

* Actual result:
  Prompt “The file you have selected is too large” but no tip size limit.

* Reproduction build:(Buri - Firefox OS V1.4 outside)
 - Gaia      17b102ee8d9a724b62285547cc5f1c5d30bfb01c
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/95be84248033
 - BuildID   20140520000201
 - Version   30.0

* Reproduction Frequency
  - 10/10
This is an UX issue, so needinfo our UX designers.
Flags: needinfo?(ofeng)
Flags: needinfo?(jelee)
Is this related to bug 990481?
Flags: needinfo?(ofeng)
Omega, I think that this issue is only about the error text that is displayed to the user.
Flags: needinfo?(ofeng)

Comment 4

5 years ago
Error text appears when 
1. attaching a single file that exceeds 300 KB, this error text appears as a dialogue box. 

 Original: The file you have selected is too large.
 Modified: The file you have selected exceeds the 300 KB size limit of a single multimedia message.

2. attaching a file that cause the MMS to exceed size limit, this error text appears as a sticky toast. 

 Original: You've exceeded the maximum size. Remove attachments or shorten the message to send it. 
 Modified: You've exceeded the 300 KB size limit of a single multimedia message. Remove attachments or shorten the message to send it.
Flags: needinfo?(ofeng)
Flags: needinfo?(jelee)
Reporter

Updated

5 years ago
Summary: [Flame][V1.4][MMS] Edit text messages, add video no size limit tips. → [Flame][v1.4][MMS] Edit text messages, add video no size limit tips.
Whiteboard: [sms-papercuts]
Assignee

Comment 5

5 years ago
Hi Julienw
Assuming this a confimed bug.
Here is the attached Patch 
Thanks
Assignee: nobody → rishav006
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8505149 - Flags: review?(felash)
Hey Matej, can you please just confirm the texts in comment 4? Thanks !
Flags: needinfo?(Mnovak)
Assignee

Comment 7

5 years ago
Yeah...one more thing.
For case 1..
Is it only for this
**Original: The file you have selected is too large
or for this also
**Original: The files you have selected are too large.

Thanks
Hey Jenny, I just want to confirm with you: we use the "configured limit - 5KB" because there is an overhead with MMS. That means the default limit is 295KB instead of 300. Would you like that we display 295 or 300 in the error message?
Flags: needinfo?(jelee)
Jenny, NI also for comment 7 :) when sharing several images from the Gallery app.
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

* You need to change the l10n key when you change the value.
* You need to take the limit value stored in Settings
* You missed some texts

Thank you :)
Attachment #8505149 - Flags: review?(felash)
(In reply to Julien Wajsberg [:julienw] from comment #6)
> Hey Matej, can you please just confirm the texts in comment 4? Thanks !

+1

Thanks!
Flags: needinfo?(Mnovak)

Comment 12

5 years ago
(In reply to Julien Wajsberg [:julienw] from comment #8)
> Hey Jenny, I just want to confirm with you: we use the "configured limit -
> 5KB" because there is an overhead with MMS. That means the default limit is
> 295KB instead of 300. Would you like that we display 295 or 300 in the error
> message?

Hi there! Although it's a little weird to show 295KB, I still think it's better to reflect the actual size limit, so let's go with 295KB. Thanks!
Flags: needinfo?(jelee)

Comment 13

5 years ago
(In reply to kumar rishav (:rishav_) from comment #7)
> Yeah...one more thing.
> For case 1..
> Is it only for this
> **Original: The file you have selected is too large
> or for this also
> **Original: The files you have selected are too large.
> 
> Thanks

The message should also include multiple selection, let's simply add a "(s)" to the original error message:
"The file(s) you have selected exceeds the 300 KB size limit of a single multimedia message."

Thanks!
Assignee

Comment 14

5 years ago
Hi Julien
Is it necessary to change the l10n key when we change the value? I asked this because i found existing one is fine. Say.. for L10n key : "files-too-large"   or "message-exceeded-max-length" . 
I didn't find a better L10n key.Please suggest some.
Thanks
Flags: needinfo?(felash)
Assignee

Comment 15

5 years ago
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Hi Julien
Updated the PR. Have a look.
Thanks
Flags: needinfo?(felash)
Attachment #8505149 - Flags: review?(felash)
(In reply to Jenny Lee from comment #13)
> (In reply to kumar rishav (:rishav_) from comment #7)
> > Yeah...one more thing.
> > For case 1..
> > Is it only for this
> > **Original: The file you have selected is too large
> > or for this also
> > **Original: The files you have selected are too large.
> > 
> > Thanks
> 
> The message should also include multiple selection, let's simply add a "(s)"
> to the original error message:
> "The file(s) you have selected exceeds the 300 KB size limit of a single
> multimedia message."
> 
> Thanks!

We don't need to add "(s)", we can change the message depending on the number of files (we already do this presently) :)
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

some minimal nits :)

please ask review when ready.

thanks again!
Attachment #8505149 - Flags: review?(felash)
Assignee

Comment 18

5 years ago
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Hi Julien
Updated the PR. Small nits are done. Hope it's okay now :)
Thanks
Attachment #8505149 - Flags: review?(felash)
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Some more nits, sorry about this :)
Attachment #8505149 - Flags: review?(felash)
Assignee

Comment 20

5 years ago
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Hi Julienw
Hope it's ok now.
Thanks
Attachment #8505149 - Flags: review?(felash)
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Please remove the unused "if" block in showMaxLengthNotice :)

And then please ask review to Steve when you're ready!
Attachment #8505149 - Flags: review?(felash)
Mentor: schung
Assignee

Updated

5 years ago
Attachment #8505149 - Flags: review?(schung)
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Hi, kumar, if we want to display number of max size, I think it might be better with your patch in bug 1014234 that could handle the unit properly. And the rest of part looks good, thanks!
Attachment #8505149 - Flags: review?(schung)
Assignee

Comment 23

5 years ago
Hi Steve
Discussed with julien and planned to do this separately. For this bug, leave it like this. For size i will file separate bug (a generic function which will return size and unit), and will do modification for bug 1014234 and bug 1018111 in this bug. 
What you say steve?

Thanks
Assignee

Updated

5 years ago
Attachment #8505149 - Flags: review?(schung)
Blocks: 1113480
Sorry for the late reply, I'm fine with leaving the file size unit to follow up(bug 1113480). Just one nit about the file size: It seems not necessary to display the decimal in the dialog(comment 13 didn't display decimal either). So still keep the review since the fixing should be really quick.
Assignee

Comment 25

5 years ago
Done.
Comment on attachment 8505149 [details] [review]
Bug 1018111 - [Flame][v1.4][MMS] Edit text messages, add video no size limit tips

Just one nit and overall looks good, thanks.
Attachment #8505149 - Flags: review?(schung) → review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/d733b56f2bc857ed84a0411f3da0d6c5a70c0b73
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S3 (9jan)
Strings in this bug landed with poor English (easy to miss with copy and paste)

When there are multiple "files", they "exceed", not "exceeds". No need to change the string ID to fix this, I guess someone could just land a quick fix without filing a new bug. Otherwise let me know and I'll file a follow-up bug.
One more nit: I know that [zero] is never used, but technically it's plural, not singular ("zero files, not zero file", that would be French or Portuguese).
Flags: needinfo?(schung)
Assignee

Comment 29

5 years ago
Hi Flod
these nits(exceed in place of exceeds for multiple files and Zero files ) will land in Bug 1113480.
Hope it's okay now?
Thanks
Flags: needinfo?(francesco.lodolo)
Yes
Flags: needinfo?(schung)
Flags: needinfo?(francesco.lodolo)
Depends on: 1134178
You need to log in before you can comment on or make changes to this bug.