only mark visible messages as read

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Conversation.java            | 15 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java   | 33 
src/main/java/eu/siacs/conversations/ui/ConversationFragment.java          | 35 
src/main/java/eu/siacs/conversations/ui/ConversationsActivity.java         |  4 
src/main/java/eu/siacs/conversations/ui/interfaces/OnConversationRead.java |  2 
5 files changed, 62 insertions(+), 27 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Conversation.java 🔗

@@ -443,7 +443,7 @@ public class Conversation extends AbstractEntity implements Blockable, Comparabl
 		return (this.messages.size() == 0) || this.messages.get(this.messages.size() - 1).isRead();
 	}
 
-	public List<Message> markRead() {
+	public List<Message> markRead(String upToUuid) {
 		final List<Message> unread = new ArrayList<>();
 		synchronized (this.messages) {
 			for (Message message : this.messages) {
@@ -451,22 +451,23 @@ public class Conversation extends AbstractEntity implements Blockable, Comparabl
 					message.markRead();
 					unread.add(message);
 				}
+				if (message.getUuid().equals(upToUuid)) {
+					return unread;
+				}
 			}
 		}
 		return unread;
 	}
 
-	public Message getLatestMarkableMessage(boolean isPrivateAndNonAnonymousMuc) {
-		synchronized (this.messages) {
-			for (int i = this.messages.size() - 1; i >= 0; --i) {
-				final Message message = this.messages.get(i);
+	public static Message getLatestMarkableMessage(final List<Message> messages, boolean isPrivateAndNonAnonymousMuc) {
+			for (int i = messages.size() - 1; i >= 0; --i) {
+				final Message message = messages.get(i);
 				if (message.getStatus() <= Message.STATUS_RECEIVED
 						&& (message.markable || isPrivateAndNonAnonymousMuc)
 						&& message.getType() != Message.TYPE_PRIVATE) {
-					return message.isRead() ? null : message;
+					return message;
 				}
 			}
-		}
 		return null;
 	}
 

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java 🔗

@@ -607,7 +607,7 @@ public class XmppConnectionService extends Service {
 						}
 						try {
 							restoredFromDatabaseLatch.await();
-							sendReadMarker(c);
+							sendReadMarker(c,null);
 						} catch (InterruptedException e) {
 							Log.d(Config.LOGTAG, "unable to process notification read marker for conversation " + c.getName());
 						}
@@ -3116,9 +3116,11 @@ public class XmppConnectionService extends Service {
 
 
 	public void markMessage(Message message, int status, String errorMessage) {
-		if (status == Message.STATUS_SEND_FAILED
-				&& (message.getStatus() == Message.STATUS_SEND_RECEIVED || message
-				.getStatus() == Message.STATUS_SEND_DISPLAYED)) {
+		final int c = message.getStatus();
+		if (status == Message.STATUS_SEND_FAILED && (c == Message.STATUS_SEND_RECEIVED || c == Message.STATUS_SEND_DISPLAYED)) {
+			return;
+		}
+		if (status == Message.STATUS_SEND_RECEIVED && c == Message.STATUS_SEND_DISPLAYED) {
 			return;
 		}
 		message.setErrorMessage(errorMessage);
@@ -3262,15 +3264,19 @@ public class XmppConnectionService extends Service {
 		return null;
 	}
 
+	public boolean markRead(final Conversation conversation, boolean dismiss) {
+		return markRead(conversation,null,dismiss).size() > 0;
+	}
+
 	public boolean markRead(final Conversation conversation) {
-		return markRead(conversation, true);
+		return markRead(conversation, null, true).size() > 0;
 	}
 
-	public boolean markRead(final Conversation conversation, boolean clear) {
-		if (clear) {
+	public List<Message> markRead(final Conversation conversation, String upToUuid, boolean dismiss) {
+		if (dismiss) {
 			mNotificationService.clear(conversation);
 		}
-		final List<Message> readMessages = conversation.markRead();
+		final List<Message> readMessages = conversation.markRead(upToUuid);
 		if (readMessages.size() > 0) {
 			Runnable runnable = () -> {
 				for (Message message : readMessages) {
@@ -3279,9 +3285,9 @@ public class XmppConnectionService extends Service {
 			};
 			mDatabaseWriterExecutor.execute(runnable);
 			updateUnreadCountBadge();
-			return true;
+			return readMessages;
 		} else {
-			return false;
+			return readMessages;
 		}
 	}
 
@@ -3298,12 +3304,13 @@ public class XmppConnectionService extends Service {
 		}
 	}
 
-	public void sendReadMarker(final Conversation conversation) {
+	public void sendReadMarker(final Conversation conversation, String upToUuid) {
 		final boolean isPrivateAndNonAnonymousMuc = conversation.getMode() == Conversation.MODE_MULTI && conversation.isPrivateAndNonAnonymous();
-		final Message markable = conversation.getLatestMarkableMessage(isPrivateAndNonAnonymousMuc);
-		if (this.markRead(conversation)) {
+		final List<Message> readMessages = this.markRead(conversation,upToUuid,true);
+		if (readMessages.size() > 0) {
 			updateConversationUi();
 		}
+		final Message markable = Conversation.getLatestMarkableMessage(readMessages, isPrivateAndNonAnonymousMuc);
 		if (confirmMessages()
 				&& markable != null
 				&& (markable.trusted() || isPrivateAndNonAnonymousMuc)

src/main/java/eu/siacs/conversations/ui/ConversationFragment.java 🔗

@@ -213,7 +213,9 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke
 
 		@Override
 		public void onScrollStateChanged(AbsListView view, int scrollState) {
-
+			if (AbsListView.OnScrollListener.SCROLL_STATE_IDLE == scrollState) {
+				fireReadEvent();
+			}
 		}
 
 		@Override
@@ -1547,11 +1549,35 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke
 			getActivity().invalidateOptionsMenu();
 		});
 		super.onResume();
+		binding.messagesView.post(this::fireReadEvent);
+	}
+
+	private void fireReadEvent() {
 		if (activity != null && this.conversation != null) {
-			activity.onConversationRead(this.conversation);
+			String uuid = getLastVisibleMessageUuid();
+			if (uuid != null) {
+				activity.onConversationRead(this.conversation, uuid);
+			}
 		}
 	}
 
+	private String getLastVisibleMessageUuid() {
+		if (binding == null) {
+			return null;
+		}
+		int pos = binding.messagesView.getLastVisiblePosition();
+		if (pos >= 0) {
+			Message message = (Message) binding.messagesView.getItemAtPosition(pos);
+			if (message != null) {
+				while(message.next() != null && message.next().wasMergedIntoPrevious()) {
+					message = message.next();
+				}
+				return message.getUuid();
+			}
+		}
+		return null;
+	}
+
 	private void showErrorMessage(final Message message) {
 		AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
 		builder.setTitle(R.string.error_message);
@@ -1884,7 +1910,8 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke
 			}
 		}
 
-		activity.onConversationRead(this.conversation);
+
+		this.binding.messagesView.post(this::fireReadEvent);
 		//TODO if we only do this when this fragment is running on main it won't *bing* in tablet layout which might be unnecessary since we can *see* it
 		activity.xmppConnectionService.getNotificationService().setOpenConversation(this.conversation);
 		return true;
@@ -2076,7 +2103,7 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke
 				this.messageListAdapter.notifyDataSetChanged();
 				updateChatMsgHint();
 				if (notifyConversationRead && activity != null) {
-					activity.onConversationRead(this.conversation);
+					binding.messagesView.post(this::fireReadEvent);
 				}
 				updateSendButton();
 				updateEditablity();

src/main/java/eu/siacs/conversations/ui/ConversationsActivity.java 🔗

@@ -592,9 +592,9 @@ public class ConversationsActivity extends XmppActivity implements OnConversatio
 	}
 
 	@Override
-	public void onConversationRead(Conversation conversation) {
+	public void onConversationRead(Conversation conversation, String upToUuid) {
 		if (!mActivityPaused && pendingViewIntent.peek() == null) {
-			xmppConnectionService.sendReadMarker(conversation);
+			xmppConnectionService.sendReadMarker(conversation, upToUuid);
 		} else {
 			Log.d(Config.LOGTAG, "ignoring read callback. mActivityPaused=" + Boolean.toString(mActivityPaused));
 		}