Please add documentation for the SMS/MMS database. It can be quite hard to find out how the database entries actually look like. It would be helpful to have the latest version of the database structures, such as messages, thread and participants, and their relationship documented in the source code. Some comments and ASCII diagrams should be sufficient.

Put this bug into backlog.

Tracing all recorded properties looks too difficult to start with. Let's extract the basic DB schema first.

Marking some (slow) progress...

slightly update

Some study outcomes.

Comment on attachment 8615162 [details] [diff] [review] Add JSdoc in MobileMessageDB.jsm Hi Bevis, Do you think the JSDoc style document helps? The generated document can be found at [1] for reference. [1] http://freesamael.github.io/jsdoc/gecko-dev/mobilemessage/MobileMessageDB.html

Comment on attachment 8615162 [details] [diff] [review] Add JSdoc in MobileMessageDB.jsm Review of attachment 8615162 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the late response. This makes the implementation self-documented. This looks great to me. :) Thanks!

Comment on attachment 8617800 [details] [diff] [review] (v2) Add JSdoc in MobileMessageDB.jsm Hi Bevis, Could you help to review the documented file? The updated jsdoc output can be found at [1] for reference. [1] http://freesamael.github.io/jsdoc/gecko-dev/mobilemessage/MobileMessageDB.html

Comment on attachment 8617800 [details] [diff] [review] (v2) Add JSdoc in MobileMessageDB.jsm Review of attachment 8617800 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the hard work of documenting this big JS file. Please see my comment inlined and also add the HOWTO instruction to generate this JS DOC at the beginning of this file. ::: dom/mobilemessage/gonk/MobileMessageDB.jsm @@ +196,5 @@ > + * | x-mms-expiry: Number | > + * | x-mms-content-location: { | > + * | uri: String | > + * | } | > + * | to: { | to: Array of { @@ +248,5 @@ > + * | participantIds: Array of Number | > + * | participantAddresses: Array of String | > + * | lastMessageId: Number | > + * | lastTimestamp: Number | > + * | lastMessageSubject: String | [MMS Only] for lastMessageSubject @@ +261,5 @@ > + > +/** > + * @typedef {Object} MobileMessageDB.ParticipantRecord > + * > + * Represents the mapping of a participant and one or multiple addresses. Represents the mapping of a participant to one or multiple addresses. (National and Int'l numbers) @@ +2497,5 @@ > }); > }, > > + /** > + * @callback MobileMessageDB.ThreadRecoardCallback ThreadRecordCallback @@ +2517,5 @@ > + * Addresses to look up with. > + * @param {boolean} aCreateParticipants > + * <code>true</code> to create participant record associated to the > + * addresses if not exist yet. > + * @param {MobileMessageDB.ThreadRecoardCallback} aCallback ThreadRecordCallback @@ +2560,5 @@ > + /** > + * @typedef {Object} MobileMessageDB.MessageRecordWrap > + * @property {MobileMessageDB.MessageRecord} messageRecord > + * The stored message record. > + */ Can you explain why we need this extra wrap of typedef? @@ +2995,5 @@ > + * @function MobileMessageDB.updateMessageDeliveryById > + * @param {string} id > + * If <code>type</code> is "messageId", it represents the message ID; > + * If <code>type</code> is "envelopeId", it represents the envelope ID, > + * which is the "message-id" in the header of an MMS message. It shall be "x-mms-transaction-id" in the header of an MMS message. @@ +3116,5 @@ > }); > }, > > + /** > + * Add participants to the MMS message record. This is to map receivers of a MMS message record to the Thread participant list with the patch in bug 1020209 applied. @@ +3470,5 @@ > > }, > > + /** > + * Update the <code>deliveryStatus</code> of a stored message record matching Update the <code>deliveryStatus</code> to the <code>receiver</code> of a stored message record matching @@ +3475,5 @@ > + * the given envelope ID. > + * > + * @function MobileMessageDB.setMessageDeliveryStatusByEnvelopeId > + * @param {string} aEnvelopeId > + * The envelope ID, which is the "message-id" in the header of an MMS It shall be the "x-mms-transaction-id" in the header of an MMS. @@ +3493,5 @@ > aDeliveryStatus, null, aCallback); > }, > > + /** > + * Update the <code>readStatus</code> of a stored message record matching the Update the <code>readStatus</code> from the <code>receiver</code> of a stored message record @@ +3498,5 @@ > + * given envelope ID. > + * > + * @function MobileMessageDB.setMessageReadStatusByEnvelopeId > + * @param {string} aEnvelopeId > + * The envelope ID, which is the "message-id" in the header of an MMS It shall be the "x-mms-transaction-id" in the header of an MMS. @@ +3672,5 @@ > }; > }); > }, > > + /** Helper to Translate NS errors to the error causes defined in MobileMessageCallback.

Comment on attachment 8632687 [details] [diff] [review] (v3) Add JSdoc in MobileMessageDB.jsm Hi Bevis, Could you help to review the updated doc? Generated doc is also updated for reference: http://freesamael.github.io/jsdoc/gecko-dev/mobilemessage/MobileMessageDB.html

Comment on attachment 8632687 [details] [diff] [review] (v3) Add JSdoc in MobileMessageDB.jsm Review of attachment 8632687 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me! Thanks for having this documented. :)