Closed Bug 1175374 Opened 5 years ago Closed Last year

Make noLog an editable attribute of the imIMessage interface.

Categories

(Instantbird :: Conversation, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: arlolra, Unassigned)

Details

Attachments

(1 file)

Attached patch log.patchSplinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.124 Safari/537.36

Steps to reproduce:

Using it here,
https://github.com/arlolra/ctypes-otr/commit/65af21f90db1860fb37b3cab0b79d7bfe72c26a2
Attachment #8623420 - Attachment is patch: true
Attachment #8623420 - Attachment mime type: text/x-patch → text/plain
Attachment #8623420 - Flags: review?(aleth)
Comment on attachment 8623420 [details] [diff] [review]
log.patch

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

::: chat/components/public/prplIMessage.idl
@@ +50,5 @@
>                                                   conversions).           */
>    /*  PURPLE_MESSAGE_NICK        = 0x0020, /**< Contains your nick.      */
>    readonly attribute boolean containsNick;
>    /*  PURPLE_MESSAGE_NO_LOG      = 0x0040, /**< Do not log.              */
> +           attribute boolean noLog;

I assume this change is an oversight?
Attachment #8623420 - Flags: review?(aleth) → review-
Comment on attachment 8623420 [details] [diff] [review]
log.patch

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

::: chat/components/src/imConversations.js
@@ +45,5 @@
>  
> +  get noLog() {
> +    return this._noLog !== null ? this._noLog : this.prplMessage.noLog;
> +  },
> +  set noLog(aBool) { this._noLog = aBool; },

Maybe if you used something like
Object.defineProperty(this, "noLog", { value: val, writable: true })
in the setter to override the getter/setter when the setter is first used, you could avoid the if clause in the getter and don't need _noLog either.
Comment on attachment 8623420 [details] [diff] [review]
log.patch

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

::: chat/components/public/prplIMessage.idl
@@ +50,5 @@
>                                                   conversions).           */
>    /*  PURPLE_MESSAGE_NICK        = 0x0020, /**< Contains your nick.      */
>    readonly attribute boolean containsNick;
>    /*  PURPLE_MESSAGE_NO_LOG      = 0x0040, /**< Do not log.              */
> +           attribute boolean noLog;

Without it, the noLog on imIMessage was not writable. I was hoping you'd know why that inheritance was happening.

::: chat/components/src/imConversations.js
@@ +45,5 @@
>  
> +  get noLog() {
> +    return this._noLog !== null ? this._noLog : this.prplMessage.noLog;
> +  },
> +  set noLog(aBool) { this._noLog = aBool; },

Should I use that same pattern for displayMessage?
Comment on attachment 8623420 [details] [diff] [review]
log.patch

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

::: chat/components/public/prplIMessage.idl
@@ +50,5 @@
>                                                   conversions).           */
>    /*  PURPLE_MESSAGE_NICK        = 0x0020, /**< Contains your nick.      */
>    readonly attribute boolean containsNick;
>    /*  PURPLE_MESSAGE_NO_LOG      = 0x0040, /**< Do not log.              */
> +           attribute boolean noLog;

The response from bz in jsapi was, "xpidl flattens everything onto the most-derived prototype, so there's no way to make that work".
On the behalf of Florian:
Closing bugs related to the Instantbird UI as WONTFIX, as the development of the standalone chat client Instantbird has stopped. Instantbird users are encouraged to migrate to Thunderbird. The user interface of instant messaging in Thunderbird will feel familiar, as the Thunderbird IM support started as a fork of Instantbird.
Status: UNCONFIRMED → RESOLVED
Closed: Last year
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.