Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve automatic reporting of incoming messages in Miranda NG #9869

Closed
wants to merge 5 commits into from

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Jul 4, 2019

Link to issue number:

Fixes #640, #1471.

Summary of the issue:

In Miranda NG, NVDA often fails to automatically report incoming messages, particularly when there are less than ten messages in a conversation.

Description of how this pull request fixes the issue:

An NVDAObjects.behaviors.liveTextHistoryMixin class, which adds an announcement history to liveText objects, has been added. A displayModelLiveTextWithoutTextInfo class has also been added, which uses the display model to receive text updates but doesn't override an object's textInfo. Instead of relying on valueChange events from the scrollbar in message windows, the Miranda NG app module now uses both of these new classes to implement automatic readout of incoming messages.

Testing performed:

Tested automatic reporting in the #flood channel on Freenode after manually starting monitoring of the liveText object (with the Python console) and verified that it is functional starting from the very first message. Pressed control+nvda+1 through 0 while the conversation log had focus and verified that messages were being read.

Known issues with pull request:

  • This implementation of MirandaMessageWindow._get_messageLog does not correctly find the control, so startMonitoring and stopMonitoring are not called when a conversation gains or loses focus. Interestingly, while the parent property of the read-only edit field properly refers to the conversation window, the control does not appear as one of its descendants.
  • The history gestures (control+NVDA+1 through 0) only execute the script when the read-only edit field has focus, but they should call the script when focused on any control in the window. I think we need to set the canPropagate property on the script to get this behavior, but even with that property set to True this is not functioning as expected.

Change log entry:

== Bug Fixes ==

== Changes for Developers ==

This commit adds a LiveTextHistoryMixin for adding a history (such as for chat applications) to LiveText. It also adds a DisplayModelLiveTextWithoutTextInfo for some controls, and begins implementation for Miranda.

It does not automatically start/stop monitoring LiveText on focusing the chat window, and the history gestures only work when the LiveText object has focus.
@codeofdusk
Copy link
Contributor Author

@feerrenrut @LeonarddeR Could you please have a look at this (particularly my implementation of _getMessageLog)?

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that this will introduce the regression that you will no longer be able to read the history if your focus is not in the actual message log. Is that correct?

@@ -365,6 +365,38 @@ def event_gainFocus(self):
def event_loseFocus(self):
self.stopMonitoring()

class LiveTextHistoryMixin(object):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHouldn't this inherrit from LiveText
I also think that you can leave out the word Mixin from the class name, as behaviors contains all mixins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would, but wouldn't that cause issues (liveText would be in the MRO twice, once as the parent of the mixin and once as a parent of the object itself)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the dynamic class logic in NVDAObjets will deal with this, but it needs to be tried out.

script_readHistoryItem.canPropagate = True

def _calculateNewText(self, newLines, oldLines):
res = super(LiveTextHistoryMixin, self)._calculateNewText(newLines, oldLines)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really relies on this being a subclass of LiveText.

Also, to follow Python 3 style, I think you can just do

Suggested change
res = super(LiveTextHistoryMixin, self)._calculateNewText(newLines, oldLines)
res = super()._calculateNewText(newLines, oldLines)

That won't work until Python 3 code is merged into master, but that will be in a short while, and I'm pretty sure @feerrenrut won't look into this beforehand.

source/NVDAObjects/window/__init__.py Outdated Show resolved Hide resolved
@codeofdusk
Copy link
Contributor Author

I'm afraid that this will introduce the regression that you will no longer be able to read the history if your focus is not in the actual message log. Is that correct?

@LeonarddeR Yes, this is correct. How can I fix this?

@LeonarddeR
Copy link
Collaborator

@codeofdusk commented on 5 Jul 2019, 07:09 CEST:

@LeonarddeR Yes, this is correct. How can I fix this?

Hmm, I need to think carefully about this. You could create a tree interceptor instead, but then you might as well just use the appModule.

You could also consider an appmodule mixin in appModuleHandler.

@LeonarddeR
Copy link
Collaborator

There is also the canPropagate property on scripts, but it still has to be set on one of the focus ancestors.

@feerrenrut
Copy link
Contributor

@codeofdusk What are your plans with this PR? Note that it is still in draft state. What is outstanding? Do you want feedback on the approach?

@codeofdusk
Copy link
Contributor Author

This PR in its current state introduces some regressions to Miranda functionality. As I don't heavily use Miranda, someone else might want to take this.

@codeofdusk codeofdusk closed this Aug 26, 2020
@feerrenrut feerrenrut added the Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. label Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. feature ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Different message buffers for each window in Miranda chat client
3 participants