show_images undocumented change

[tomsommer]

This

roundcubemail/program/actions/mail/index.php

Line 868 in a94c2a3

case 3: // all my contacts

Does not match with this:

roundcubemail/config/defaults.inc.php

Line 1283 in 0217dec

// 1 - Allow from my contacts (all writeable addressbooks + collected senders and recipients)

or this:

roundcubemail/program/actions/settings/index.php

Line 618 in d7338a4

$input->add($rcmail->gettext('fromtrustedsenders'), 1);

In the past show_images=1 was all contacts, now it's been changed to "only trusted"? In any case this has broken the previous show_images=1 functionality - which is really, really bad.

Don't know if it's because this does not work as intended:

roundcubemail/program/actions/mail/index.php

Line 873 in a94c2a3

$type |= rcube_addressbook::TYPE_RECIPIENT | rcube_addressbook::TYPE_WRITEABLE;

[tomsommer]

Also, when show_images=1 the "add to contacts" button is still showed, even though the sender IS in an addressbook - perhaps related to #7961 or to the issue above -- and clicking "add to contacts" generates an "Already exists in addressbook"-error

[alecpl]

The description in the config file is indeed mixed. I fixed that.

In 1.4 show_images=1 was using the default addressbook, which was a problem if you had more addressbooks. That was one of the reasons for the change.

Now show_images=1 uses the "Trusted sender" addressbook only. So, the expected behavior is:

• with show_images=1, check if the sender is in the trusted senders addressbook, if not display "Always allow from..." .
  button, which will store the sender in trusted senders addressbook.
• with show_images=3, check in all writeable addressbooks and trusted senders and collected recipients, if not found display "Always allow from..." button, which will store the sender in the configured default addressbook (fallbacks to the first writeable source).

There is also collected_senders option which allows you to tell Roundcube where to store trusted senders and for example disable the trusted senders addressbook from appearing in the UI, but the concept of "trusted senders" is still there.

So, tell me what is not working as described?

[tomsommer]

In 1.4 show_images=1 was the default addressbooks (I think it was even "all addressbooks")? Now it's not? now it requires to be in TYPE_TRUSTED_SENDER? So it changes functionality on upgrade. 3 is the new 1.

Or maybe it's the broken "add to contacts" and "allow" buttons when running show_images=1 that is breaking the usage.

[alecpl]

show_images=1 was never "all addressbooks", it was the default addressbook only. So, neither 1 nor 3 is the same as before, but I see the migration problem. That being said, I'm not sure we'll change anything at this point.

I don't see what problem do you have with "Add to contacts" button, it always added contacts to the default addressbook, and should act the same way in 1.5.

[tomsommer]

But now show_images=1 is rcube_addressbook::TYPE_TRUSTED_SENDER, before it was the default addressbook?

Old function:

function rcmail_check_safe($message)
{
    global $RCMAIL;

    if (!$message->is_safe
        && ($show_images = $RCMAIL->config->get('show_images'))
        && $message->has_html_part()
    ) {
        switch ($show_images) {
        case 1: // known senders only
            // get default addressbook, like in addcontact.inc
            $CONTACTS = $RCMAIL->get_address_book(-1, true);

            if ($CONTACTS && $message->sender['mailto']) {
                $result = $CONTACTS->search('email', $message->sender['mailto'], 1, false);
                if ($result->count) {
                    $message->set_safe(true);
                }
            }

            $RCMAIL->plugins->exec_hook('message_check_safe', array('message' => $message));
            break;

        case 2: // always
            $message->set_safe(true);
            break;
        }
    }

    return !empty($message->is_safe);
}

[tomsommer]

If you set show_images=1 and the contact is in the addressbook, clicking the "Always allow from" just generates an error-popup

[alecpl]

Ok, I see where's the issue. The addcontact request should contain a _source argument that would tell it to look for existing contacts only in the trusted senders source. The code already exist in the request handler (program/actions/mail/addcontact.php), but is missing on the client-side.

[tomsommer]

I really think this is wrong: 7e4bac4 Changing functionality like this. show_images=1 does not work like it did in 1.4.

[davidearl]

Re you observations in #8624, changing the trusted senders to personal addresses does indeed solve the problem.

But to find this broken after an upgrade with no obvious clue as to what to do about it is a nasty surprise. It is not at all clear to me why the fact a contact is in my address book should prevent me from making them an exception for showing images. Surely if the contact is already there it should just update the contact to allow the exception rather than tryin to add a new contact to a different address book.

Anyway, my immediate problem is now dealt with, thank you.

[alecpl]

I fixed the problem described in #8264.

I'll consider swapping option 1 and 3 of show_images.

[davidearl]

Thank you!

[alecpl]

Option 1 and 3 have been swapped. Fixed.