(In reply to Patrick Cloke [:clokep] from comment #12) > > I think we also want to clear out `this._roomList` to reset the state. See > https://searchfox.org/comm-central/rev/ > 2131bede09ff0e7273bc77f669150491cd0d7685/chat/protocols/irc/irc.jsm#2258-2262 > Right. > The core code guarantees the account gets disconnected: > https://searchfox.org/comm-central/source/chat/components/src/imAccounts. > jsm#760-762 Ohh, got it. This multiple call of reportDisconnected of causing some error so needed the check of if (this.disconnected) further in the code. > > I don't think we can just make this async -- the caller doesn't know it is > async so won't properly await. > The caller doesn't need to wait since connect function is not returning anything. We just need to call this.reportConnected() whenever we are prepared. We are doing it here. Without await, the store will not startup on time and createClient will throw an error. We just need synchronous behavior in the "connect" function itself, we don't need to worry about the caller function. > I'm not convinced this is the best thing to use here... I think we should > use something based on the account ID instead. This means that even if > someone deletes the account and recreates it we are guaranteed to be using a > different storage. > > Maybe something like: `dbName = "chat:matrix:" + this.imAccount.id;`? I > think that will do something like "chat:matrix:1". Looking at how we > generate entries for the password manager might be useful. Okay cool. > > What does this change do? > [opts.useAuthorizationHeader = false] Set to true to use Authorization header instead of query param to send the access token to the server. > > Do you know if this comment is still valid? > Yes, we don't need it right now. > > Is this just for debugging or do we still need it? > It will generate a message and we want to show it to the user. That's why. This all will changes once we update this writeConv function and show the right message according to the respective event.
Bug 1614025 Comment 13 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Patrick Cloke [:clokep] from comment #12) > > I think we also want to clear out `this._roomList` to reset the state. See > https://searchfox.org/comm-central/rev/ > 2131bede09ff0e7273bc77f669150491cd0d7685/chat/protocols/irc/irc.jsm#2258-2262 > Right. > The core code guarantees the account gets disconnected: > https://searchfox.org/comm-central/source/chat/components/src/imAccounts. > jsm#760-762 Ohh, got it. This multiple call of reportDisconnected of causing some error so needed the check of if (this.disconnected) further in the code. > > I don't think we can just make this async -- the caller doesn't know it is > async so won't properly await. > The caller doesn't need to wait since connect function is not returning anything. We just need to call this.reportConnected() whenever we are prepared. We are doing it here. Without await, the store will not startup on time and createClient will throw an error. We just need synchronous behavior in the "connect" function itself, we don't need to worry about the caller function. > I'm not convinced this is the best thing to use here... I think we should > use something based on the account ID instead. This means that even if > someone deletes the account and recreates it we are guaranteed to be using a > different storage. > > Maybe something like: `dbName = "chat:matrix:" + this.imAccount.id;`? I > think that will do something like "chat:matrix:1". Looking at how we > generate entries for the password manager might be useful. Okay cool. > > What does this change do? > [opts.useAuthorizationHeader = false] Set to true to use Authorization header instead of query param to send the access token to the server. > > Do you know if this comment is still valid? > Yes, we don't need it now. > > Is this just for debugging or do we still need it? > It will generate a message and we want to show it to the user. That's why. This all will changes once we update this writeConv function and show the right message according to the respective event.
(In reply to Patrick Cloke [:clokep] from comment #12) > > I think we also want to clear out `this._roomList` to reset the state. See > https://searchfox.org/comm-central/rev/ > 2131bede09ff0e7273bc77f669150491cd0d7685/chat/protocols/irc/irc.jsm#2258-2262 > Right. > The core code guarantees the account gets disconnected: > https://searchfox.org/comm-central/source/chat/components/src/imAccounts. > jsm#760-762 Ohh, got it. This multiple calls of reportDisconnected were causing some error so needed the check of ``if (this.disconnected)`` further in the code. > > I don't think we can just make this async -- the caller doesn't know it is > async so won't properly await. > The caller doesn't need to wait since the "connect" function is not returning anything. We just need to call ``this.reportConnected()`` whenever we are prepared. We are doing it here. Without await, the store will not startup on time and ``createClient`` function will throw an error. We just need synchronous behavior in the "connect" function itself, we don't need to worry about the caller function. > I'm not convinced this is the best thing to use here... I think we should > use something based on the account ID instead. This means that even if > someone deletes the account and recreates it we are guaranteed to be using a > different storage. > > Maybe something like: `dbName = "chat:matrix:" + this.imAccount.id;`? I > think that will do something like "chat:matrix:1". Looking at how we > generate entries for the password manager might be useful. Okay cool. > > What does this change do? > [opts.useAuthorizationHeader = false] Set to true to use Authorization header instead of query param to send the access token to the server. > > Do you know if this comment is still valid? > Yes, we don't need it now. > > Is this just for debugging or do we still need it? > It will generate a message and we want to show it to the user. That's why. This all will changes once we update this writeConv function and show the right message according to the respective event.