[B2G][1.4][Flame][MMS] Message report displays message size in KB at minimum

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mkruml, Assigned: rishav_, Mentored)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(b2g-v1.3 unaffected, b2g-v1.4 affected)

Details

(Whiteboard: [flame-1.4-exploratory][lang=js][sms-papercuts])

Attachments

(2 attachments)

35.69 KB, image/png
Details
PR
46 bytes, text/x-github-pull-request
steveck
: review+
Details | Review
Description:
The 'message size' section of the MMS report is listed in KB at the least, which is effective for the traditional MMS that includes a photo or video file. However, adding a subject line to an SMS will transfer it into MMS, and the subsequent text message will be registered as a fraction of a kilobyte, instead as a measurement in Bytes.

Repro Steps:
1) Update a Flame to BuildID: 20140520000201
2) Turn on message reports for MMS in Settings
3) Navigate to SMS app and create a text message, attaching a subject line
4) After sending the message, observe the message size in the message report:

Actual:
The minimum message size is displayed in KB, resulting in purely text messages to often display as 0.00 KB

Expected:
The minimum message size is displayed in Bytes (B) to give an accurate measurement of file size when the message only contains text.

Occurs on both Buri and Flame 1.4, does not occur in 1.3 due to lacking functionality

1.4 Environmental Variables:
Device: Flame 1.4 MOZ
BuildID: 20140520000201
Gaia: 17b102ee8d9a724b62285547cc5f1c5d30bfb01c
Gecko: 95be84248033
Version: 30.0
Firmware Version: v10f-3

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140520000201
Gaia: 17b102ee8d9a724b62285547cc5f1c5d30bfb01c
Gecko: 95be84248033
Version: 30.0
Firmware Version: v1.2-device.cfg

Repro frequency: 100%
See attached: screenshot: 2014-05-21-14-13-47.png
see [1] for a possible useful function to be used here. We clearly don't need higher than MB though.

[1] https://github.com/mozilla-b2g/gaia/blob/6088316e189e02cdf0ead472e64531c426b6f73e/apps/system/js/app_install_manager.js#L528-L538
information.js already computes KB/MB, it would need to compute B too.
Mentor: felash
Whiteboard: [flame-1.4-exploratory] → [flame-1.4-exploratory][lang=js]
Whiteboard: [flame-1.4-exploratory][lang=js] → [flame-1.4-exploratory][lang=js][sms-papercuts]
Hey, I would like to work on this bug. I am new to open source and would like someone to guide me through this bug!
Hey, I would like to work on this bug. I am new to Firefox OS and would greatly appreciate the help to guide me through this bug!
Hi gauravmittal/Goutam ,welcome to join this project! if you are new to I'll strongly suggest you study some doc[1] first. It could help you have a brief general about the gaia codebase and how to run it. If you facing any problem while developing feel free to ask here or through IRC :gaia channel for instant feedback, thanks!

[1] https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia
Assignee: nobody → rishav006
Posted file PR
Hi Julienw
Due to their inactivity on bug ,it Seems they are not working on this.
So I am assigning it to myself and Here is the PR. 
Have a look on it. Hope it should be fine. Treeherder also green. Please let me know if i m missing anything.
Thanks
Attachment #8505363 - Flags: review?(felash)
Status: NEW → ASSIGNED
Comment on attachment 8505363 [details] [review]
PR

Please move the function that calculates the sizes to utils.js and do a proper unit test for its different cases.

Also please change the l10n key when you change the value.

Thanks !
Attachment #8505363 - Flags: review?(felash)
Comment on attachment 8505363 [details] [review]
PR

Hi Julien
PR updated.
Thanks
Attachment #8505363 - Flags: review?(felash)
Comment on attachment 8505363 [details] [review]
PR

Looks good at first glance, I'll defer the final review to Steve
Attachment #8505363 - Flags: review?(felash) → review?(schung)
Comment on attachment 8505363 [details] [review]
PR

Some suggestions on github, mostly good but I think we can simply shows integer in Byte since it's the smallest unit already.
Attachment #8505363 - Flags: review?(schung)
Comment on attachment 8505363 [details] [review]
PR

Hi Steve
Hope it's good now.
Thanks
Attachment #8505363 - Flags: review?(schung)
Comment on attachment 8505363 [details] [review]
PR

Hi Kumar, thanks for the patch and it looks good now. But only one concern is the try-server-hook didn't launch with your latest commit(Server might be busted previously). Could you please try to send the commit again to push the try-server-hook? If it still fail to trigger the hook, please create another new pull request for it. Sorry for the inconvenience :(
Attachment #8505363 - Flags: review?(schung) → review+
Done :) 
Can i set checkin-needed now?

Thanks
When the try is finished and green :)

Note that you should also add "r=schung" at the end of the first line of your commit log!
Keywords: checkin-needed
In master : 838f354c9110fe8bd5467dcbc832708a9b0c5958
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.