Skip to content

Fix sign in issues

Joshua Goins requested to merge (removed):fix-signin into master

Closes #40 (closed). Right now there are two separate issues:

AbstractAccount::setInstanceUri exits way too early

The crux of the issue is that m_client_id is not being set, but why? See abstractaccount.cpp:

void AbstractAccount::setInstanceUri(const QString &instance_uri)
{
    if (m_instance_uri == instance_uri) {
        return;
    }

...

As you can see, there is an early exit whenever this is true - but this is always true so it always exits early and the client id is never filled... making login impossible :-(

However just removing this bit doesn't solve the issue entirely! see account.cpp:

Account::Account(const QString &name, const QString &instanceUri, QNetworkAccessManager *nam, bool ignoreSslErrors, QObject *parent)
    : AbstractAccount(parent, name, instanceUri)
...

Yes, MockAccount and Account are children of AbstractAccount, and setInstanceUri is being called inside of the constructor - hence once it starts doing HTTP requests using the AbstractAccount::post() virtual method - C++ rightfully complains. This MR also moves it out of the constructor in AbstractAccount and now it's the job of the Account and MockAccount to call it. I don't have experience with how the mock account is set up, so please give me some guidance if I was supposed to change something there too.

Message filtering infinite loop

So now sign in works correctly... until QMessageFilterContainer does an infinite loop. Why? The reason is deceptively simple and so is the solution. Let's start by covering where this code was taken from, which seems to be Dr. Konqi? :-) I double checked and the source code is the same, so it's safe to say they didn't encounter any bugs with it.

However, this is weird. It's like the handler is calling into itself for some reason, but that shouldn't be happening. If we instead log when the handler is being created (using std::cout and not qDebug of course) it's now obvious how this happens:

/home/josh/Development/tokodon/cmake-build-debug/bin/tokodon
called QMessageFilterContainer()
called QMessageFilterContainer()
Loading any accounts from settings.
XXX

Qt documentation sheds some light onto how qInstallMessageHandler works too:

Installs a Qt message handler which has been defined previously. Returns a pointer to the previous message handler.

So what seems to happen is the global seems to be initialized twice (one for each thread i'm guessing, but not confirmed) and they're actually overwriting each other's handlers. So the one that comes after is actually calling the same message filter, icky. To fix this is actually simple, and once again Qt documentation comes to the rescue:

To restore the message handler, call qInstallMessageHandler(0).

So I just called that in the constructor, just to make sure we're restoring the original message handler:

QMessageFilterContainer::QMessageFilterContainer()
{
    qInstallMessageHandler(0);

    handler = qInstallMessageHandler([](QtMsgType type, const QMessageLogContext &context, const QString &msg) {
        s_messageFilter->handler(type, context, s_messageFilter->filter(msg));
    });
}

Now with both of these fixes, Tokodon now starts up fine and I can add new accounts - yay! Now I'm worried that Dr. Konqi might run into the same issue eventually ;-)

Edited by Joshua Goins

Merge request reports