check.is_inside_email is not reliable anymore

[sieppl]

I am using gmail.js for multiple years now with our Google Workspace. It seems the function is not reliable anymore as gmail is not including a mail_id in every message div.

    api.check.is_inside_email = function() {
        if(api.get.current_page() !== "email" && !api.check.is_preview_pane()) {
            return false;
        }

        var items = document.querySelectorAll(".ii.gt .a3s");
        var ids = [];

        for(var i=0; i<items.length; i++) {
            var mail_id = items[i].getAttribute("class").split(" ")[2];
            if(mail_id !== "undefined" && mail_id !== undefined) { //THIS IS SOMETIMES UNDEFINED FOR SOME MESSAGES
                ids.push(items[i]);
            }
        }

        return ids.length > 0;
    };

Example of a message without a mail_id:

(Working) Example of a message with a mail_id:

Suggested Fix:
Remove the extra check for mail_id, but I guess it was added for a reason before.

[sieppl]

Working version of the function:

    api.check.is_inside_email = function() {
        if(api.get.current_page() !== "email" && !api.check.is_preview_pane()) {
            return false;
        }

        return true;
    };

[josteink]

Thanks for the report and investigation.

Can you submit this as a PR and I’ll try to test the changes to see if they work for me.

[josteink]

Just adding to this thread by saying I've tested this now, and I can reproduce the same issue.

For some emails, the function returns false when it clearly should return true. I can see how this creates other issues down the line.

[Becky-Brooks]

Just chiming in here to confirm that we're seeing this as well. We use this function as a trigger for a feature, and found recently that it was not reliably returning true as it once did. We also found that the issue seems predicated on the mail_id being provided, though not sure why Gmail would make this change.

[josteink]

As the one one who last touched this code, I'm going to assume the reason we're checking for the ID property was a way to make extra, doubly sure and not accidentally fall on the other side of the fence:

That is, we also want to make sure we don't return true when we should be returning false.

If someone could volunteer to test the new implementation in different scenarios and see if it works as expected (ie, doesn't introduces regression), I'd be much more comfortable pushing and merging the proposed fix.