Messages chat polling freezes the browser
Steps to reproduce
- Type the text I want to search in the Nextcloud unified search box
- Click one of the search result
Expected behaviour
The expected behaviour should be my focus will be on the search result I click. But it turned out I was still in the same place. I think the potential reason might be the text messages are too long.
Actual behaviour
The focus was still at the same place
Talk app
Talk app version: (see apps admin page: /index.php/settings/apps)
11.3.1
Browser
Operating system: macOS
Browser name: Chrome
Browser version: 91.0.4472.164
Browser log
Server configuration
Operating system: Ubuntu/RedHat/...
Web server: Apache/Nginx
Database: MySQL/Maria/SQLite/PostgreSQL
PHP version: 7.2/7.3/7.4
Nextcloud Version: (see admin page)
List of activated apps:
If you have access to your command line run e.g.:
sudo -u www-data php occ app:list
from within your server installation folder
Nextcloud configuration:
If you have access to your command line run e.g.:
sudo -u www-data php occ config:list system
from within your Nextcloud installation folder
Server log (data/nextcloud.log)
Insert your server log here
It seems related to this statement?
https://github.com/nextcloud/spreed/blob/90e0c72e69fbcf17c20c58b68eaea967b8b77bd6/src/components/MessagesList/MessagesList.vue#L426-L428
No offense here. I understand this feature may be still in progress. But wouldn't you think if we cannot focus on the exact position if that text is not present on the current page, isn't the unified search engine basically not really effective in this use case?
@jimlinntu there's some logic missing that would need to find out how far to scroll up to load previous pages before focussing.
So if you search for a term that is present in the latest pages, the focus should work. Otherwise you'll run into this issue.
This indeed makes search not that useful currently
Now thinking, the current logic uses the "last unread message" as the separator between "fetch one page of old messages" and "fetch all new messages". Maybe when the hash in the URL contains a message, we could use that message id instead.
It also means that if the message in question is a million pages in the past, the "fetch all new messages" logic will take a while. We'll also need to fix the "fetch all new messages" logic to be paginated, basically just fetch 1-2 pages of new messages and stop there until the user scrolls down.
Thanks for your explanation!
Can you elaborate more about what are the main challenges of "fetch 1-2 pages of messages in question and fetch more only when the use scroll down or up"? Are these challenges caused by backend database or purely frontend challenges?
With the current logic, if we don't fetch all messages to the bottom, it will conflict with the logic that "long polls for new messages". Should new messages appear in the chat if not scrolled to bottom ? Probably not I guess.
And possibly more potential issues, like sticky mode when scrolled to the bottom of that chat window. There might be more I'm not aware of.
It's not trivial as there's a risk to break existing logic which might need to be redone/rethought.
Can you point out:
- Where is the "long polls for new messages" code? (and as far as you know, how Facebook Messenger handles
- What is the sticky mode and where is the code of that?
I am just interested in that.
Perhaps if given more information, I might be able to help with this feature.
If you have more time, do you mind explaining:
- what's the current implementation in Nextcloud Talk different from Facebook Messenger in terms of the search feature?
- Is the implementation mainly inside: https://github.com/nextcloud/spreed/tree/master/src/components/MessagesList ?
@jimlinntu you can start reading here: https://github.com/nextcloud/spreed/blob/master/src/components/MessagesList/MessagesList.vue#L450
this is what happens when the message list needs an update due to switching conversations
it will first fetch the old messages getOldMessages based on the indicators "first know message id" and "last known message id".
Then later below it calls lookForNewMessages which:
- if the last know message id is not the last message from the chat: it will pull all messages (or up to 200) after "last known message id"
- if the last known message id is the same as last message, it means all messages were fetched already, so the backend will keep the connection open in a "long poll" mode. as soon as new messages come in, it will send them back and close the connection. The long poll has a timeout.
Also, if you look more closely, you can see that lookForNewMessages will be called again and again to make sure we always have the long poll in place.
Regarding similarities of search with Facebook Messenger, I don't know, sorry.
Suggestion, instead of loading 3 years worth of message, with click on the search result you might
- show an overlay with the target message with possibility to load more earlier or later messages.
- have ↑ integrated into the current message view and display a "load more" button between the messages from the result and presently visible recent messages. This bites a but with the load-on-scroll-up behaviour.
This would prevent loading a lot of stuff, memory consumption in the browser etc, and would also be more clear and manageable for the user.
show an overlay with the target message with possibility to load more earlier or later messages.
Sounds like a good workaround for the time being?!
I'm using links to talk messages especially for references in deck. Even if the message is part of the recent discussion, focussing of the message doesn't seem to work reliable (message out of view, message not highlighted). Is this already covered by this issue or should I open a separate one?
I am glad to see there is some update after a year.
Frankly, I just wanna it to behave like Facebook messenger to increase the usability of this app
API endpoint for this was added in #8717
Also the frontend is now using the new endpoint when the chat has no messages loaded for that conversation yet (due to technical limitations).
Also the frontend is now using the new endpoint when the chat has no messages loaded for that conversation yet (due to technical limitations).
One way we could tackle this would be to clear the loaded messages when we try to open a conversation from search. So if messages are already loaded for that conversation and the message to display is not in the loaded messages, clear the message store for that conversation so that the context api can load it correctly. I think we would benefit quite a lot from fixing and the drawback of clearing the locally loaded messages is neglect able in contrast.
Since the introduced solution is merged with PR mentioned above:
- If message is not presented in store yet, we're fetching it and its context https://github.com/nextcloud/spreed/pull/9897/commits/92a63328470719a160cc66d910c9a230b58caf0c :heavy_check_mark:
- As long as the old logic of polling new messages exists, we keep fetching new messages, request by request, until we reach the current date (so for 3-years old context, it will be full list of messages introduced for these 3 years, which will definitely kill a browser) :warning:
- One of the mentioned solutions above is close to the proposal I'd like to put there: based on reference values of conversation / context message (lastActivity, id, simple count, e.t.c.) we could estimate the limit of new messages, which will be fetched after the context loaded, and after that stop the process, disable long poll for new messages, and preserve a chat in "history mode", until it will be reloaded / rejoined / scrolled to the end (with fetching context of last known chat message)
- If we're doing this within already opened chat, could be wise to purge previously loaded messages, to not have a 3-year gap between current conversation and a history part, or preserve it, but disable filling the gap automatically (example from GitHub):
Will keep an issue open until questions are solved