Closed StCyr closed 5 years ago
StCyr
commented
5 years ago Just to make it clear, steps to reproduce are the following:
StCyr
commented
5 years ago I've added some console log, and it seems that there's some race condition: mail seems to try to load the next/previous message, then mail load the deleted message again.
In the log hereunder "1632" is the next/previous message while I can't find "1610" back in the EnvelopeList. And, while I had to redact informations from the 1632 message for privacy, the 1610 message doesn't contain much information except its date. So, I believe 1610 is a reference to the deleted message.
in onEnvelopeDeleted EnvelopeList.vue:121
Trying to show next/previous envelope EnvelopeList.vue:141
fetching message Message.vue:122
1-SU5CT1g=-1632 Message.vue:123
action.js fetchMessage 1 SU5CT1g= 1632 actions.js:442:10
/index.php/apps/mail/api/accounts/1/folders/SU5CT1g%3D/messages/1632 MessageService.js:63:9
fetching message Message.vue:122
1-SU5CT1g=-1610 Message.vue:123
action.js fetchMessage 1 SU5CT1g= 1610 actions.js:442:10
/index.php/apps/mail/api/accounts/1/folders/SU5CT1g%3D/messages/1610 MessageService.js:63:9
Object { id: 1632, from: (1) […], to: (2) […], cc: [], bcc: [], fromEmail: "xxxxxxxxxxx", subject: "Fwd: xxxxxxxxxxxxxxxxx", date: "21 mai 2019", dateInt: 1558423442, dateIso: "2019-05-21T09:24:02+02:00", … }
actions.js:445:11
User navigated away, loaded message won't be shown nor flagged as seen Message.vue:130
message removed actions.js:479:12
Object { id: 1610, from: [], to: [], cc: [], bcc: [], fromEmail: null, subject: "", date: "24 mai 2019", dateInt: 1558709316, dateIso: "2019-05-24T14:48:36+00:00", … }
actions.js:445:11
back in **Message.vue**Hi @ChristophWurst ,
Could it be that this issue comes from a conflict between the following 2 code snippets?
The Vue.delete() call in function removeEnvelope in src/store/mutations.js:
removeEnvelope(state, {accountId, folder, id}) {
const envelopeUid = accountId + '-' + folder.id + '-' + id
const idx = folder.envelopes.indexOf(envelopeUid)
if (idx < 0) {
console.warn('envelope does not exist', accountId, folder.id, id)
return
}
folder.envelopes.splice(idx, 1)
const unifiedAccount = state.accounts[UNIFIED_ACCOUNT_ID]
unifiedAccount.folders
.map(fId => state.folders[fId])
.filter(f => f.specialRole === folder.specialRole)
.forEach(folder => {
const idx = folder.envelopes.indexOf(envelopeUid)
if (idx < 0) {
console.warn('envelope does not exist in unified mailbox', accountId, folder.id, id)
return
}
folder.envelopes.splice(idx, 1)
})
Vue.delete(folder.envelopes, envelopeUid)
},
the this.$router.push call in function onEnvelopeDeleted in src/components/EnvelopeList.vue:
onEnvelopeDeleted(envelope) {
const envelopes = this.envelopes
const idx = this.envelopes.indexOf(envelope)
if (idx === -1) {
console.debug('envelope to delete does not exist in envelope list')
return
}
let next
if (idx === 0) {
next = envelopes[idx + 1]
} else {
next = envelopes[idx - 1]
}
if (!next) {
console.debug('no next/previous envelope, not navigating')
return
}
// Keep the selected account-folder combination, but navigate to a different message
// (it's not a bug that we don't use next.accountId and next.folderId here)
this.$router.push({
name: 'message',
params: {
accountId: this.$route.params.accountId,
folderId: this.$route.params.folderId,
messageUid: next.uid,
},
})
},Anyway, this onEnvelopeDeleted function doesn't seem right at all: This is not because we delete an email that we want to navigate to its direct neighbour; We might be reading another email atm and just want to delete an email in the list without switching to the deleted email's neighbour.
I believe this "navigate to neighbour" function should better be implemente in the removeEnvelope function and should handle both the case when we are currently reading the email we want to delete and the case when we are currently reading another email than the one we want to delete.
What do you think?
Cyrille
ChristophWurst
commented
5 years ago If the current email is deleted we should navigate. If an other email is deleted then the current message should stay.
StCyr
commented
5 years ago Yes, sure: this functionality must stay.
But don’t you think the call to this.$store.push is redundant with the Vue.delete statement?
Le 29 mai 2019 à 16:51, Christoph Wurst notifications@github.com a écrit :
If the current email is deleted we should navigate. If an other email is deleted then the current message should stay.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
ChristophWurst
commented
5 years ago I believe this "navigate to neighbour" function should better be implemente in the
removeEnvelopefunction and should handle both the case when we are currently reading the email we want to delete and the case when we are currently reading another email than the one we want to delete.
removeEnvelope is the store part. The navigation is the responsibility of the view. I would not mix the two.
But don’t you think the call to this.$store.push is redundant with the Vue.delete statement?
I don't see the conflict in the snippets shown. Could you please elaborate?
StCyr
commented
5 years ago But don’t you think the call to this.$store.push is redundant with the Vue.delete statement?
I don't see the conflict in the snippets shown. Could you please elaborate?
I think the this.$store.push statement triggers an update of the view (to display the next/previous message) but that it gets latter overriden by the Vue.delete statement, therefore returning in a state where it displays the just deleted email.
Though, in the mean time, I could test this hypothesis and infirm it unfortunately.
StCyr
commented
5 years ago So, here's a nicer log when I delete the currently displayed email:
Showing next/previous envelope (in onEnvelopeDeleted) 2-SU5CT1g=-17740 EnvelopeList.vue:140
Deleting envelope from vue (in mutation.js) 1-SU5CT1g=-1703 mutations.js:162:10
message removed 1703 actions.js:504:12
When fetching message 2-SU5CT1g=-17740 Message.vue:131
route parameter messageUid is 1-SU5CT1g=-1703 Message.vue:132
So, user navigated away and loaded message won't be shown nor flagged as seenAnd, here's the log when I delete another email than the one displayed:
Showing next/previous envelope (in onEnvelopeDeleted) 2-SU5CT1g=-17743 EnvelopeList.vue:140
Deleting envelope from vue (in mutation.js) 1-SU5CT1g=-1758 mutations.js:162:10
TypeError: "this.$refs.menu is undefined"
unFocus ncvuecomponents.js:131
VueJS 3
vue.runtime.esm.js:1888
message removed 1758 actions.js:504:12
When fetching message 2-SU5CT1g=-17743 Message.vue:131
route parameter messageUid is 1-SU5CT1g=-1758 Message.vue:132
So, user navigated away and loaded message won't be shown nor flagged as seenIn both cases (when I delete the currently displayed email or not), the end result is the same: the deleted email is shown.
StCyr
commented
5 years ago some more logs:
Showing next/previous envelope (in onEnvelopeDeleted) 1-SU5CT1g=-1768 EnvelopeList.vue:140
Deleting envelope from vue (in mutation.js) 1-SU5CT1g=-1769 mutations.js:162:10
route params have changed
Object { name: "message", meta: {}, path: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1768", hash: "", query: {}, params: {…}, fullPath: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1768", matched: (1) […] }
Object { name: "message", meta: {}, path: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1769", hash: "", query: {}, params: {…}, fullPath: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1769", matched: (1) […] }
Message.vue:106
fetchMessage 1-SU5CT1g=-1768 Message.vue:126
route params have changed
Object { name: "message", meta: {}, path: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1769", hash: "", query: {}, params: {…}, fullPath: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1769", matched: (1) […] }
Object { name: "message", meta: {}, path: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1768", hash: "", query: {}, params: {…}, fullPath: "/accounts/0/folders/aW5ib3g=/message/1-SU5CT1g=-1768", matched: (1) […] }
Message.vue:106
fetchMessage 1-SU5CT1g=-1769 Message.vue:126
fetched message 1768 actions.js:445:12
When fetching message 1-SU5CT1g=-1768 Message.vue:133
route parameter messageUid is 1-SU5CT1g=-1769 Message.vue:134
So, user navigated away and loaded message won't be shown nor flagged as seen Message.vue:135
message removed 1769 actions.js:507:12
fetched message 1769argh! this drives me crazy!
@ChristophWurst , any idea why route params are changed twice in this case? Or, how to hunt this change's origin?
ChristophWurst
commented
5 years ago Had a look at the code. It's indeed strange that there are two route changes because this.$router.push is only called once, isn't it? could you check that? I don't see any other code that could trigger that.
StCyr
commented
5 years ago I've also looked hard at the code and couldn't find neither why the route's params where changed twice....
Now, I'm seing another strange deletion behaviour: In some (most) draft folders, when I delete a draft, the envelope and the context menu ("Mark unread" and "Delete") stays on the screen after the message has been deleted.
And, here are the console logs I can get:
14:29:50.429 Showing next/previous envelope (in onEnvelopeDeleted) 1-SU5CT1guRHJhZnRz-249 EnvelopeList.vue:140
14:29:50.433 Deleting envelope from vue (in mutation.js) 1-SU5CT1guRHJhZnRz-265 mutations.js:162:21
14:29:50.468 NewMessageDetail.vue created NewMessageDetail.vue:107
14:29:50.469 NewMessageDetail.vue fetchMessage NewMessageDetail.vue:137
14:29:50.469 action.js fetchMessage 265 actions.js:442:10
14:29:50.485 Content Security Policy: The page's settings blocked the loading of a resource at eval ("script-src"). mutations.js:162:2
14:29:50.942 message removed 265 actions.js:508:12
14:29:50.977 fetched message 265 actions.js:446:12
14:29:50.980 todo: handle draft data NewMessageDetail.vue:53
14:29:50.983
Object { id: Getter & Setter, accountId: Getter & Setter, name: Getter & Setter, emailAddress: Getter & Setter, imapHost: Getter & Setter, imapPort: Getter & Setter, imapUser: Getter & Setter, imapSslMode: Getter & Setter, signature: Getter & Setter, smtpHost: Getter & Setter, … }
Composer.vue:352I think the problem doesn't occur in draft folders created via the nextcloud mail app.
Expected behavior
When I delete a message from the Envelope's app-content-list-item-menu (the 3 little dots of the Envelope in the EnvelopeList), mail should automaticaly show the next message in the list
Actual behavior
Instead of showing the next message in the list, the deleted message is still shown
Mail app
Mail app version: 0.15.1
Mailserver or service: OVH + Gmail
Server configuration
Operating system: Debian Stretch
Web server: Apache
Database: MariaDB
PHP version: 7.3
Nextcloud Version:16.0.1
Client configuration
Browser: Firefox Quantum Latest (67.0)
Operating system: Windows 10 64bits