Would this somehow improve #9035? If so perhaps you may consider adding an example app module fixing one of the bugs referenced there as part of this pr?

@lukaszgo1 commented on 6 aug. 2019 13:02 CEST:

Would this somehow improve #9035? If so perhaps you may consider adding an example app module fixing one of the bugs referenced there as part of this pr?

Yes, it definitely would, though you would still have to subclass the used textInfo and implement the right selectionCondition.

I've experimented with saving the selection condition on the object, but that has its downsides.

Here is an example I"m currently working on it is for a corporate application though:

class KingMenuTextInfo(DisplayModelTextInfo):
	selectionCondition = [{
		'color': [RGB(255, 255, 255)],
	}]

This example treats all white text as being selected.

@feerrenrut Is this on your radar for 2020.3 perhaps? Currently it is required to copy entire _getSelectionOffsets for every Displaymodel textinfo in an appmodule and change just a few lines if the default criteria for selection are not used in a given program.

Hi @feerrenrut I'm quite interested in having this merged sooner than later so if @LeonarddeR can't work on this further I'd be happy to take this. What would help however would be to have this PR reviewed - perhaps it can be approved as is and no further actions are necessary.

@lukaszgo1 That would be great! Just reviewing this quickly again, the query idea (Field.evaluateQuery) is complicated, it would be good if it had unit tests.

It seems to me that the intention with this PR is not to change user visible behavior, but to give greater flexibility for specializing behavior for certain apps. If this is correct it would be good to clarify that in the description. Also, it would be good to think about how to verify that the existing behavior hasn't changed.

@michaelDCurran Do you have anything to add to this?

If you wish to take this on, create a new branch based on this one and open a new PR. Comment the PR number here and then I'll close this one.

the query idea (Field.evaluateQuery) is complicated.

I stole the logic from UIAUtils.createUIAMultiPropertyCondition

It seems to me that the intention with this PR is not to change user visible behavior, but to give greater flexibility for specializing behavior for certain apps. If this is correct it would be good to clarify that in the description.

This is correct.

Since this is hard for us to test across a wide range of applications, and there are likely lots of users depending on the current behavior with legacy enterprise applications, I'd like to see this a feature flag wrapping the new behavior.

I'd also like to see some automated tests for this.

Since this is hard for us to test across a wide range of applications, and there are likely lots of users depending on the current behavior with legacy enterprise applications, I'd like to see this a feature flag wrapping the new behavior.

There won't be any difference in behaviour with this pr, it only majorly increases the customizability of the selection detection logic.

So the intention is that there is no change in behavior, but customizability is easier? Customization via add-ons I assume?

I was thinking if there is a bug, it would be good to be able revert to old code. Though looking at this more, I guess there is only singular logic built into NVDA. There may be bugs exposed by add-ons, but the addon can be disabled.

If the intention here is to make this easy for add-ons or in built appmodules to customize, it would be useful for it to be included in the description.

• PASS: Translation comments check.
• FAIL: Unit tests. See test results for more information.
• FAIL: Lint check. See test results for more information.
• FAIL: System tests. See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/ktg187h5mhp1njq3/artifacts/output/nvda_snapshot_pr10040-23421,2673e5fd.exe

See test results for failed build of commit 2673e5fd25

I'm taking this again.

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/aecky4768jnrfoq3/artifacts/output/nvda_snapshot_pr10040-23422,44ac4749.exe

See test results for failed build of commit 44ac474905

To test this code I've converted the way in which I detect selection in my add-on for Becky! Internet Mail and while it is much easier to provide selection criteria when the given text info uses different colors for showing selection this PR broke NVDA's ability to detect selections spanning multiple lines in DisplayModel based text infos (only first line of the selection is reported when querying for selected text). I haven't investigated further as of yet since @LeonarddeR told us they want to continue working on this.

I have time nor interest to work on this further. @lukaszgo1 would you be interested to take this further?

Closing as abandoned