vladimiry / ElectronMail

Unofficial ProtonMail Desktop App
GNU General Public License v3.0
1.5k stars 96 forks source link

Incorrect unread emails counters rendered in the Proton UI #529

Closed RobertKirchner9 closed 2 years ago

RobertKirchner9 commented 2 years ago

This is a not-very-serious-but-still-annoying bug. I'm running ElectronMail 5.0.1 on Linux Mint Mate 20.3. As you can see in the attached screenshot, there is a 2 to the right of "All mail" in the folder menu on the left, ostensibly indicating 2 unread messages. But this is spurious. There are no unread messages in any of my folders. Even when I click the "unread" filter in "All mail" itself, nothing shows up. These spurious unread message numbers show up next to various folders regularly. I can't clear them, even by logging out and in again. And the ElectronMail tray icon shows spurious unread message numbers as well. Screenshot at 2022-06-24 12-02-59

vladimiry commented 2 years ago

ostensibly indicating 2 unread messages

3

This unread indication is not part of the ElectronMail project, but https://github.com/ProtonMail/WebClients, so it's irrelevant here.

prestr commented 2 years ago

@vladimiry This is a bug with the app. On both Windows and macOS, I recently signed out of my account and back in (to get Proton Drive access) and ever since doing that I have this issue.

Steps to reproduce:

  1. If you're logged into ElectronMail already log out and sign back in with the proton.me entry point, then quit the app.
  2. Have 1 unread message in your inbox then launch ElectronMail v5.0.1
  3. View the message and notice the Inbox no longer shows a (1) unread message but if you look at All Mail there is still a (1) unread message indicated

If you open All Mail, there's nothing unread. Same thing happens with Labels, if the message in the inbox had a label the label still shows a (1).

Restarting ElectronMail clears it up until you get another new message.

prestr commented 2 years ago

To add to this. If you mark a message as Unread the Inbox shows (1) unread but the macOS dock icon doesn't update nor does the Tray icon on Windows.

vladimiry commented 2 years ago

Do you have the "local store" option enabled in the app for the mail account?

Restarting ElectronMail clears it up until you get another new message.

You could also click "Unload" menu item opened by right mouse click on the account handle button. This action triggers the page reload, and so the proton-web-client gets reloaded too.

I recently signed out of my account and back in (to get Proton Drive access) and ever since doing that I have this issue.

To make the Drive service work, the app forces the "account" auth scope when a new session gets open (so you had to re-sign in into the account), as discussed in https://github.com/vladimiry/ElectronMail/issues/522#issuecomment-1156989727. This could potentially introduce side effects as Proton might differentiate some logic based on the auto scope type. There were no other recent major changes in the app, so it's either this auth scope related issue or the Proton's web client change. It's yet to be investigated.

prestr commented 2 years ago

Local store is disabled.

Clicking Unload does refresh, All Mail updates and my Dock icon unread also updates correctly.

prestr commented 2 years ago

If its of any help...

I went back to v5.0.0 and the issue persists, then tried v4.14.0 and it's working fine. So it seems like the issue started happening with v5.0.0.

vladimiry commented 2 years ago

Can confirm that if the "local store" disabled, making the 1 unread email read doesn't wipe the "1" tray indicator nor the "all mail (1)" counter. I normally go with a "local store" option enabled, so didn't notice the issue before. If the "local store" enabled, the tray icon is fine (at least if you make the mails read via the "local store" means) as in this mode tray icon under status update has its own logic, but the "all mail (1)" in the proton's app is still there. Beside if I remove the mail, select the "all mail" folder in the proton-web-client, the summary mails count value displayed at the right page side doesn't get updated with -1 value (so remains static). Everything is fine in the "local store" mode as it handles the mails independently ("all mails" counter is fine, same as the tray update, etc).

I went back to v5.0.0 and the issue persists, then tried v4.14.0 and it's working fine. So it seems like the issue started happening with v5.0.0.

Same user session was used or you signed-in (you can see the open session list in the account's settings app, then go "security" section ...)?

vladimiry commented 2 years ago

There were no other recent major changes in the app, so it's either this auth scope related issue or the Proton's web client change.

I went back to v5.0.0 and the issue persists, then tried v4.14.0 and it's working fine. So it seems like the issue started happening with v5.0.0.

So if the issue was still there in 5.0.0 then it's not related to the "account" auth scope forcing and the remaining issue cause then is the proton web client change itself. If so, then it's likely related to some caching (I've noticed they started using Redux for some needs not so long time ago).

prestr commented 2 years ago

Same user session was used or you signed-in (you can see the open session list in the account's settings app, then go "security" section ...)?

Between switching versions I removed my Proton account from the app then added it back.

vladimiry commented 2 years ago

Between switching versions I removed my Proton account from the app then added it back.

Ok, so "account" auth scope forcing is still an option.

vladimiry commented 2 years ago

Here is the debugging progress. In the app, the /v4/events/<event-id> API request gives the data without a *Counts-like fields (like MessageCounts field/prop for example). But if I run in-browser version (v5.0.5.4 at the moment), those fields are there. This is likely the invalid counter values rendering issue cause (and also the tray update if the "local store" feature is disabled, as the app simply doesn't see "unread" value update).

vladimiry commented 2 years ago

Ok, so "account" auth scope forcing is still an option.

In both cases, the app and in-browser client, the session title is "Proton Account for web". So the cause doesn't seem to be the "account" scope forcing related.

The mail-api subdomain use and /api prefix dropping was introduced since v5.0.0, so going to drop this logic and see what happens, since the in-browser version still uses the officially deprecated /api-prefixed API requests.

vladimiry commented 2 years ago

Since v5.0.0 the app switched to <app-type>-api API subdomain use + dropped the /api prefix. This was named as a proffered way. But the in-browser/live-prod version still sits on the previously used API addresses scheme, and so we got the following API address difference (same for the Tor address):

proton client type app API_URL in-browser API_URL
mail https://mail-api.proton.me https://mail.proton.me/api
calendar https://calendar-api.proton.me https://calendar.proton.me/api
drive https://drive-api.proton.me https://drive.proton.me/api
account settings https://account-api.proton.me https://account.proton.me/api
vpn settings https://account-api.proton.me https://account.proton.me/api

I thought maybe the issue cause lies here and switched back to the originally used API address scheme, but the /v4/events/ API still doesn't return the *Counts fields (particularly the MessageCounts one). So now I wonder how to make the proton mail client to act being bundled in the app the same way as it works in-browser.

@bartbutler / @mmso I guess there is some logic at the backend that based on some criteria decides to include the *Count fields into the /v4/events/ response since we have the *Count fields attached if I run it in browser, but no fields if I run the app. Can it be the auth type difference (SSO vs non-SSO/old way used on the app) or maybe something else?

vladimiry commented 2 years ago

Just reverted the x-pm-appversion: web-account@... header forcing for the /auth/* API call needed for the Drive access fix and got the *Count-like fields filled for the /v4/events/<event-id> API call.

So it's a blocker here, as we either get access to the Drive service by forcing the "Proton Account for web" session via x-pm-appversion: web-account@... header for /auth/* API call, or we get *Count-like fields filled for the /v4/events/<event-id> API call which fixes the unread counters rendering, but without having access to the Drive service. @bartbutler would appreciate some feedback.

vladimiry commented 2 years ago

@prestr if there won't be a workaround/solution found like in a few days, I tend to completely drop the Drive service use possibility via the app and release a new version.

prestr commented 2 years ago

@prestr if there won't be a workaround/solution found like in a few days, I tend to completely drop the Drive service use via the app and release a new version.

Sounds good. Using Drive was a nice to have but myself (and maybe most people) don't use it often so I'd rather have the Mail fix. Thanks for looking into this.

superfreeman1989 commented 2 years ago

I'm having similar issue. Unread badge persists after deleting new email. Disabling and enabling the account removed the badge.

vladimiry commented 2 years ago

Disabling and enabling the account removed the badge.

You could also click "Unload" menu item opened by right mouse click on the account handle button. This action triggers the page reload, and so the proton-web-client gets reloaded too.

bartbutler commented 2 years ago

So a looked at the code and found this comment:

FIXME: Deprecated, should use the MessageCounts and ConversationCounts queries explicitly

Basically, counts are included for the WebMail appversion header automatically for legacy reasons but not for others because it's inefficient.

Web mail should fix this by requesting the counts explicitly. I will ask them to do this. In the mean time, if you want the message or conversation counts or both you just need to include MessageCounts and/or ConversationCounts query parameters (values are not important) and they'll be put in the response.

vladimiry commented 2 years ago

In the mean time, if you want the message or conversation counts or both you just need to include MessageCounts and/or ConversationCounts query parameters (values are not important) and they'll be put in the response.

Thanks. Going to try this later today.

vladimiry commented 2 years ago

In the mean time, if you want the message or conversation counts or both you just need to include MessageCounts and/or ConversationCounts query parameters (values are not important) and they'll be put in the response.

Can confirm that 743229c fixes the issue. Going to publish a new release within a few days.