Merge pull request #1002 from jpavonabian/fix-blueski-thread-dupes

Fix duplicated posts in Bluesky threads and bogus list position in direct messages

src/controller/blueski/handler.py

@@ -587,16 +587,28 @@ class Handler:
                 display_name = g(author, "displayName") or g(author, "display_name")
         label = handle or display_name or _("Unknown")
         title = _("Conversation with {0}").format(label)
-        
+        # The buffer name identifies the buffer and keys its entry in session.db, so
+        # it has to be derived from the post instead of the author. Two threads from
+        # the same account would otherwise share a single database entry.
+        buffer_name = "%s-conversation" % (uri,)
+        account_name = buffer.session.get_name() if hasattr(buffer.session, "get_name") else None
+
+        # If this thread is already open, go to it rather than opening it twice.
+        if account_name != None and controller.search_buffer(buffer_name, account_name):
+            index = controller.view.search(buffer_name, account_name)
+            if index is not None:
+                controller.view.change_buffer(index)
+            return
+
         from pubsub import pub
         pub.sendMessage(
             "createBuffer",
             buffer_type="conversation",
             session_type="blueski",
             buffer_title=title,
-            parent_tab=controller.view.search(buffer.session.get_name(), buffer.session.get_name()) if hasattr(buffer.session, "get_name") else None,
+            parent_tab=controller.view.search(account_name, account_name) if account_name != None else None,
             start=True,
-            kwargs=dict(parent=controller.view.nb, name=title, session=buffer.session, uri=uri, sound="search_updated.ogg")
+            kwargs=dict(parent=controller.view.nb, name=buffer_name, session=buffer.session, uri=uri, sound="search_updated.ogg")
         )
 
     def open_timeline(self, controller, buffer, default="posts"):

src/controller/buffers/blueski/chat.py

@@ -257,12 +257,25 @@ class ConversationListBuffer(BaseBuffer):
             self.buffer.list.insert_item(False, *row)
 
         # Restore selection if possible
+        restored = False
         if selected_key is not None:
             for idx, convo in enumerate(new_db):
                 key = self.get_convo_id(convo) or self._get_members_key(convo)
                 if key == selected_key:
                     self.buffer.list.select_item(idx)
+                    restored = True
                     break
+        # This buffer rebuilds its list on every refresh and does not go through
+        # put_items_on_list(), so nothing would be focused on the first load or
+        # whenever the previously selected conversation is gone. Screen readers
+        # then announce the position as "0 of n", so focus an item ourselves.
+        if restored == False:
+            total = self.buffer.list.get_count()
+            if total > 0:
+                if self.session.settings["general"].get("reverse_timelines", False):
+                    self.buffer.list.select_item(0)
+                else:
+                    self.buffer.list.select_item(total-1)
 
         # Sound and auto-read only when something actually changed
         if new_count > 0:
@@ -397,7 +410,11 @@ class ConversationListBuffer(BaseBuffer):
                 try:
                     count = buffer_obj.buffer.list.get_count()
                     if count > 0:
-                        msg = _("{0}, {1} of {2}").format(title, buffer_obj.buffer.list.get_selected()+1, count)
+                        selected = buffer_obj.buffer.list.get_selected()
+                        if selected < 0:
+                            selected = count-1
+                            buffer_obj.buffer.list.select_item(selected)
+                        msg = _("{0}, {1} of {2}").format(title, selected+1, count)
                     else:
                         msg = _("{0}. Empty").format(title)
                 except Exception:

src/controller/buffers/blueski/timeline.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import logging
+import wx
 import output
 from .base import BaseBuffer
 from wxUI.buffers.blueski import panels as BlueskiPanels
@@ -237,6 +238,9 @@ class Conversation(BaseBuffer):
         self.type = "conversation"
         self.root_uri = kwargs.get("uri")
         self.sound = "search_updated.ogg"
+        # Set while a refresh is in flight. This buffer rebuilds itself from scratch
+        # on every call, so two overlapping runs would show every post twice.
+        self.loading = False
 
     def create_buffer(self, parent, name):
         self.buffer = BlueskiPanels.HomePanel(parent, name)
@@ -245,11 +249,15 @@ class Conversation(BaseBuffer):
     def start_stream(self, mandatory=False, play_sound=True):
         if not self.root_uri:
             return 0
-        api = self.session._ensure_client()
+        if self.loading == True:
+            return 0
+        self.loading = True
         try:
+            api = self.session._ensure_client()
             res = api.app.bsky.feed.get_post_thread({"uri": self.root_uri, "depth": 100, "parentHeight": 100})
             thread = getattr(res, "thread", None)
             if not thread:
+                self.loading = False
                 return 0
 
             def g(obj, key, default=None):
@@ -277,14 +285,42 @@ class Conversation(BaseBuffer):
                     traverse(r)
 
             traverse(thread)
+
+            # A post must only show up once, no matter how the thread is shaped.
+            seen = set()
+            unique_items = []
+            for item in final_items:
+                uri = g(item, "uri")
+                if uri:
+                    if uri in seen:
+                        continue
+                    seen.add(uri)
+                unique_items.append(item)
+            final_items = unique_items
+        except Exception as e:
+            log.error("Error fetching thread: %s", e)
+            self.loading = False
+            return 0
+        # The list control may only be touched from the GUI thread, and start_stream
+        # is also called from the update timer, which runs in a background thread.
+        if wx.IsMainThread():
+            self._rebuild_list(final_items, play_sound)
+        else:
+            wx.CallAfter(self._rebuild_list, final_items, play_sound)
+        return len(final_items)
+
+    def _rebuild_list(self, items, play_sound=True):
+        """Replace the contents of this buffer with the given thread items."""
+        try:
             self.session.db[self.name] = []
             self.buffer.list.clear()
             # Don't use process_items() because it applies reverse logic.
             # Conversations should always be chronological (oldest first).
-            return self._add_items_chronological(final_items, play_sound)
+            self._add_items_chronological(items, play_sound)
         except Exception as e:
-            log.error("Error fetching thread: %s", e)
-            return 0
+            log.error("Error building thread: %s", e)
+        finally:
+            self.loading = False
 
     def _add_items_chronological(self, items, play_sound=True):
         """Add items in chronological order (oldest first) without reverse logic."""
@@ -295,7 +331,17 @@ class Conversation(BaseBuffer):
         relative_times = self.session.settings["general"].get("relative_times", False)
         show_screen_names = self.session.settings["general"].get("show_screen_names", False)
 
+        # Unlike process_items(), this method does not deduplicate on its own, so
+        # guard against a post reaching the list more than once.
+        added = 0
+        seen = set()
         for item in items:
+            uri = item.get("uri") if isinstance(item, dict) else getattr(item, "uri", None)
+            if uri:
+                if uri in seen:
+                    continue
+                seen.add(uri)
+            added += 1
             self.session.db[self.name].append(item)
             post = self.compose_function(item, self.session.db, self.session.settings,
                                          relative_times=relative_times,
@@ -311,7 +357,7 @@ class Conversation(BaseBuffer):
         if play_sound and self.sound and not self.session.settings["sound"]["session_mute"]:
             self.session.sound.play(self.sound)
 
-        return len(items)
+        return added
 
 
 class LikesBuffer(BaseBuffer):

src/controller/mainController.py

@@ -493,6 +493,9 @@ class Controller(object):
                 except ValueError:
                     commonMessageDialogs.unauthorized()
                     return
+                # This buffer has already retrieved its items, so start_buffers()
+                # must not start it a second time.
+                buffer.needs_init = False
             self.buffers.append(buffer)
             if parent_tab == None:
                 log.debug("Appending buffer {}...".format(buffer,))
@@ -1180,6 +1183,31 @@ class Controller(object):
 #        except:
 #            pass
 
+    def announce_buffer_position(self, buffer, with_account=False):
+        """ Speaks the current buffer name plus the position within its list.
+
+        If the list holds items but none of them is focused yet, this focuses one
+        before speaking. Otherwise get_selected() returns -1 and we would announce
+        a position of "0 of n", which does not exist."""
+        try:
+            count = buffer.buffer.list.get_count()
+            if count == 0:
+                raise ValueError("Empty buffer")
+            selected = buffer.buffer.list.get_selected()
+            if selected < 0:
+                reverse = False
+                if getattr(buffer, "session", None) != None:
+                    reverse = buffer.session.settings["general"].get("reverse_timelines", False)
+                selected = 0 if reverse else count-1
+                buffer.buffer.list.select_item(selected)
+            if with_account == True:
+                msg = _(u"%s. %s, %s of %s") % (buffer.account, self.view.get_buffer_text(), selected+1, count)
+            else:
+                msg = _(u"%s, %s of %s") % (self.view.get_buffer_text(), selected+1, count)
+        except:
+            msg = _(u"%s. Empty") % (self.view.get_buffer_text(),)
+        output.speak(msg, True)
+
     def left(self, *args, **kwargs):
         buff = self.view.get_current_buffer_pos()
         buffer = self.get_current_buffer()
@@ -1193,11 +1221,7 @@ class Controller(object):
         while self.get_current_buffer().invisible == False: self.skip_buffer(False)
         buffer = self.get_current_buffer()
         if self.showing == True: buffer.buffer.set_focus_in_list()
-        try:
-            msg = _(u"%s, %s of %s") % (self.view.get_buffer_text(), buffer.buffer.list.get_selected()+1, buffer.buffer.list.get_count())
-        except:
-            msg = _(u"%s. Empty") % (self.view.get_buffer_text(),)
-        output.speak(msg, True)
+        self.announce_buffer_position(buffer)
 
     def right(self, *args, **kwargs):
         buff = self.view.get_current_buffer_pos()
@@ -1212,11 +1236,7 @@ class Controller(object):
         while self.get_current_buffer().invisible == False: self.skip_buffer(True)
         buffer = self.get_current_buffer()
         if self.showing == True: buffer.buffer.set_focus_in_list()
-        try:
-            msg = _(u"%s, %s of %s") % (self.view.get_buffer_text(), buffer.buffer.list.get_selected()+1, buffer.buffer.list.get_count())
-        except:
-            msg = _(u"%s. Empty") % (self.view.get_buffer_text(),)
-        output.speak(msg, True)
+        self.announce_buffer_position(buffer)
 
     def next_account(self, *args, **kwargs):
         if not self.accounts:
@@ -1243,11 +1263,7 @@ class Controller(object):
         self.view.change_buffer(buff)
         buffer = self.get_current_buffer()
         if self.showing == True: buffer.buffer.set_focus_in_list()
-        try:
-            msg = _(u"%s. %s, %s of %s") % (buffer.account, self.view.get_buffer_text(), buffer.buffer.list.get_selected()+1, buffer.buffer.list.get_count())
-        except:
-            msg = _(u"%s. Empty") % (self.view.get_buffer_text(),)
-        output.speak(msg, True)
+        self.announce_buffer_position(buffer, with_account=True)
 
     def previous_account(self, *args, **kwargs):
         if not self.accounts:
@@ -1274,11 +1290,7 @@ class Controller(object):
         self.view.change_buffer(buff)
         buffer = self.get_current_buffer()
         if self.showing == True: buffer.buffer.set_focus_in_list()
-        try:
-            msg = _(u"%s. %s, %s of %s") % (buffer.account, self.view.get_buffer_text(), buffer.buffer.list.get_selected()+1, buffer.buffer.list.get_count())
-        except:
-            msg = _(u"%s. Empty") % (self.view.get_buffer_text(),)
-        output.speak(msg, True)
+        self.announce_buffer_position(buffer, with_account=True)
 
     def go_home(self):
         buffer = self.get_current_buffer()