Filter LTR and RTL marks from list view items by dkager · Pull Request #5751 · nvaccess/nvda

dkager · 2016-02-13T18:28:52Z

Update: see #5751 (comment) for a better summary of what this does.

Related issues: #5729, #4364

This extends NVDAObjects.IAccessible.TrayClockWClass to also cover recent versions of Windows. It does two things:

Filters out the Unicode character U+200E (left-to-right mark). Question: should U+200F (right-to-left) also be filtered?
Discards the object name (see the commit for details).

In #5729 I said I would rather implement this in the Explorer app module, but in the end that got a bit confusing so I changed my mind. All suggestions and comments are appreciated as always.

Tested on Windows 7 and Windows 10. I tried to test on XP but the launchers I built crashed after initializing comtypes.

… included in the value on recent versions of Windows. This character is usually not defined in the liblouis braille tables. re nvaccess#5729

On Windows 7 the name is "Clock", same as the role. On Windows 10 the name equals the value. Both situations cause repeated announcements in NVDA. On Windows XP the clock has no value and the name only contains the time. Use this for its value since that is what is used on newer versions of Windows. re nvaccess#4364

dkager · 2016-02-24T13:56:18Z

Ugh, this character (U+200e) is also shown in the Details view of Windows Explorer in the Date modified column. Would be nice to get rid of it there too since it gets in the way of reading file dates.

dkager · 2016-02-27T14:46:53Z

Yet another case of this problem shows up in the Windows 10 Programs and Features section in Control Panel. This again is a date, in this case the installation date. The difference is that here it's a standard list item instead of a proper multi-column view like in the Details view of Windows Explorer.

It looks like this was addressed before, see ListItemWithoutColumnSupport._get_value. The ListItem class inherits from it, but there the value is explicitly set to None and a dynamic value is exposed through _get_name. This should probably also deal with the annoying LTR/RTL marks. Hopefully that is the last case I will find... An updated patch will follow.

josephsl · 2016-02-27T16:31:44Z

Hi, is this still the case on Windows 7/8.1 with 2016.1? Thanks.

From: Davy Kager [mailto:notifications@github.com]
Sent: Saturday, February 27, 2016 6:47 AM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] Improve reading of the clock in the taskbar (#5751)

Yet another case of this problem shows up in the Windows 10 Programs and Features section in Control Panel. This again is a date, in this case the installation date. The difference is that here it's a standard list item instead of a proper multi-column view like in the Details view of Windows Explorer.

It looks like this was addressed before, see ListItemWithoutColumnSupport._get_value. The ListItem class inherits from it, but there the value is explicitly set to None and a dynamic value is exposed through _get_name. This should probably also deal with the annoying LTR/RTL marks. Hopefully that is the last case I will find... An updated patch will follow.

—
Reply to this email directly or view it on GitHub #5751 (comment) .

dkager · 2016-02-27T16:34:48Z

I don’t know about 7, but it is on 10.

From: josephsl [mailto:notifications@github.com]
Sent: Saturday, February 27, 2016 17:32
To: nvaccess/nvda nvda@noreply.github.com
Cc: Davy Kager mail@davykager.nl
Subject: Re: [nvda] Improve reading of the clock in the taskbar (#5751)

Hi, is this still the case on Windows 7/8.1 with 2016.1? Thanks.

From: Davy Kager [mailto:notifications@github.com]
Sent: Saturday, February 27, 2016 6:47 AM
To: nvaccess/nvda <nvda@noreply.github.com mailto:nvda@noreply.github.com >
Subject: Re: [nvda] Improve reading of the clock in the taskbar (#5751)

Yet another case of this problem shows up in the Windows 10 Programs and Features section in Control Panel. This again is a date, in this case the installation date. The difference is that here it's a standard list item instead of a proper multi-column view like in the Details view of Windows Explorer.

It looks like this was addressed before, see ListItemWithoutColumnSupport._get_value. The ListItem class inherits from it, but there the value is explicitly set to None and a dynamic value is exposed through _get_name. This should probably also deal with the annoying LTR/RTL marks. Hopefully that is the last case I will find... An updated patch will follow.

—
Reply to this email directly or view it on GitHub #5751 (comment) .

—
Reply to this email directly or view it on GitHub #5751 (comment) . https://github.com/notifications/beacon/AC9y8WLppP2g_d9HTb6N5QP-w0BsldvPks5pocbzgaJpZM4HZsGx.gif

…ne in ListItemWithoutColumnSupport.

…dates in Windows Explorer. For example, Date modified.

…riable className.

dkager · 2016-02-28T17:50:20Z

The commits I just pushed should fix all cases of this problem reported thus far. Tested on Windows 10 and XP so far.

josephsl · 2016-04-03T08:11:39Z

+		if value is None:
+			#On XP there is no value, fall back to using the name.
+			return super(TrayClockWClass, self).name
+		#Strip Unicode left-to-right and right-to-left marks.


I think referencing a ticket might help future devs to find out why the below code is necessary.

Good point, will add that.

From: josephsl [mailto:notifications@github.com]
Sent: Sunday, April 3, 2016 10:12
To: nvaccess/nvda nvda@noreply.github.com
Cc: Davy Kager mail@davykager.nl
Subject: Re: [nvaccess/nvda] Improve reading of the clock in the taskbar (#5751)

In source/NVDAObjects/IAccessible/init.py #5751 (comment) :

""" def _get_role(self): return controlTypes.ROLE_CLOCK

def _get_name(self):

#The name contains redundant information that is available either in the role or the value.

return None

def _get_value(self):

value = super(TrayClockWClass, self).value

if value is None:

#On XP there is no value, fall back to using the name.

return super(TrayClockWClass, self).name

#Strip Unicode left-to-right and right-to-left marks.

I think referencing a ticket might help future devs to find out why the below code is necessary.

—
You are receiving this because you authored the thread.
Reply to this email directly or https://github.com/nvaccess/nvda/pull/5751/files/19b600d36da6a6f2557663dc605a251fad2086c6#r58306141 view it on GitHub https://github.com/notifications/beacon/AC9y8fJ8BqF7uVD9EksepuqFARlO3zU-ks5pz3ZAgaJpZM4HZsGx.gif

josephsl · 2016-04-03T08:20:15Z

Did some code review. Looks fine to me (so far). If we can get a confirmation that this is working in Windows 7 and 8.x, then I'd like to request incubating this pull request. Thanks.

dkager · 2016-06-07T17:34:29Z

Recently I found out that the RSS feed view in IE11 also contains these marks. However, I think it goes too far to catch those too. Instead, I would like to suggest that we discard these characters in standard Windows widgets as this PR aims to do, and leave the rest up to braille table developers. There is always going to be just one more case we missed.

derekriemer · 2016-12-01T02:20:55Z

Would braille dictionaries (Something I didn't get around to doing) fix this?

dkager · 2016-12-01T10:13:42Z

Would braille dictionaries (Something I didn't get around to doing) fix this?
Possibly, but apparently this work-around has already been implemented elsewhere (in MSAA lists if I remember right). The problem also shows up in speech if you go charcter by character, or at least I would assume so.

One complication is that the clock changes almost with every iteration of Windows, including Windows 10 Anniversary Update.

dkager · 2017-01-18T15:41:21Z

@feerrenrut Any thoughts on usefulness/priority of this?

jcsteh · 2017-01-20T03:01:13Z

I'm now somewhat unclear as to what this PR does; i.e. exactly what controls it touches and what it does. For example, you mention discarding the object name, but at least on Windows 10 build 14971, discarding the name of the clock in the system tray would mean we lose the information altogether. Can you please provide a summary of what this fixes from a user perspective? Thanks.

dkager · 2017-01-20T10:19:09Z

I'm now somewhat unclear as to what this PR does; i.e. exactly what controls it touches

It touches:

System tray clock controls, which already had a class defined in NVDAObjects.
List view items, which already filter out the LTR/RTL marks.
Windows Explorer Details view rows, where these characters also show up.

As such the PR is a misnomer. It really is about filtering the LTR/RTL marks found in e.g. the clock. These are usually not spoken, but they are brailled and also spoken when you go character-by-character.
It should be noted that there are more of these symbols (U+202A, U+202B, etc). Some of these also appear in Windows, but the PR doesn't touch them.

example, you mention discarding the object name, but at least on Windows 10 build 14971, discarding the name of the clock in the system tray would mean we lose the information altogether.

The code uses the object's value but falls back to the name if there is no value. I think I did this to avoid having to filter LTR/RTL marks from both name and value, depending on the OS in use.
This is only done for the TrayClockWClass window class. In Windows 10 Anniv Update the clock is a button instead of a clock widget, because that totally makes sense, so this code doesn't apply at all.

Can you please provide a summary of what this fixes from a user perspective?

For example, in Windows Explroer, this changes the way a date is brailled from:
'\x200e'18-'\x200e'Mar-'\x200e'2016 '\x200f''\x200e'12:53
To:
18-Mar-2016 12:53

This reverts commit 9293131.

… that is included in the value on recent versions of Windows. This character is usually not defined in the liblouis braille tables. re nvaccess#5729" This reverts commit 6a16a24.

dkager · 2017-04-25T14:41:24Z

@jcsteh looks like this PR is quite old. I simplified it a bit. Now it only touches list view item and items in Windows Explorer details view.
The comments in the code should explain the motivation. Could we decide on the faith of this humble patch?

LeonarddeR · 2017-06-26T09:46:28Z

I think this looks good

LeonarddeR · 2017-10-12T07:38:12Z

 		if not value:
 			return None
-		#Some list view items in Vista can contain annoying left-to-right and right-to-left indicator characters which really should not be there.
+		#Some list view items in Vista and later can contain annoying left-to-right and right-to-left indicator characters which really should not be there.


Vista support is no more, so you could consider changing this to 7...

For historical tracking, I'd rather keep it this way. It is good to know when this behavior started.

LeonarddeR · 2017-10-12T07:40:48Z

+
+	def _get_value(self):
+		value = super(UIProperty, self).value
+		return value.replace(u'\u200E','').replace(u'\u200F','')


Might as well use translate here, as @jcsteh suggested

LeonarddeR · 2017-10-12T07:41:50Z

 			if role == controlTypes.ROLE_LIST:
 				clsList.remove(List)
 			clsList.insert(0, StartButton)
+			return


Where's this one coming from?

IIRC this short-circuits the function to avoid doing a bunch of checks that won't lead to any more code being executed.

Can you add a comment here to explain this? When maintaining this code, it will be important to know that the early return here is purely an optimisation, and not functional. I guess something along the lines of: "Because the windowClass is start, and role is either list or button, none of the checks after this will apply, so we can return early"

dkager · 2018-02-01T17:27:08Z

Can you add a comment here to explain this? When maintaining this code, it will be important to know that the early return here is purely an optimisation, and not functional.

Yes, I can add a comment, but there are a number of similar checks with return above that one. I think we need a generic comment at the top of the function.

dkager

Addressed the review comments.

dkager · 2018-02-01T17:48:10Z

-				clsList.insert(0,MultitaskingViewFrameWindow)
-			elif obj.windowClassName=="MultitaskingViewFrame" and role==controlTypes.ROLE_LISTITEM:
-				clsList.insert(0,MultitaskingViewFrameListItem)
+			elif uiaClassName=="MultitaskingViewFrame":


@feerrenrut @LeonarddeR While this seems logical I'd like your expert opinion.

I'm happy with this as long as you manually confirm that uiaClassName is equal to obj.windowClassName when looking at the "MultitaskingViewFrame"

I dug a bit deeper in the code and reverted this change. However, while reading the function I also tried to improve readability and consistency of code style a bit.

If you found out why, could you add a comment here about why this one uses windowClassName rather than uiaClassName?

feerrenrut · 2018-02-05T05:23:51Z

 class AppModule(appModuleHandler.AppModule):

 	def chooseNVDAObjectOverlayClasses(self, obj, clsList):
+		# Optimization: return early to avoid comparing class names and roles that will never match.


The problem with this, is that we don't know which returns are purely optimisation, and which are functional, if any. Perhaps there is no easy way to convey this information, and someone intending to modify this function will have to work it out. If there was a way that was not overly verbose, it would reduce the maintenance cost of this function.

feerrenrut · 2018-02-05T05:59:34Z

 		states.discard(controlTypes.STATE_SELECTED)
 		return states
+
+CHAR_LTR_MARK = u'\u200E'


It would be good if this file and the other could refer to the same constants.

The explorer app module imports SysListView32, but it seems ugly to put the constants there. Still, I see no other options that look much better. Should we go for it?

Yeah, I agree that isn't much better. Really, a unicode constants file would be handy. Leave it as is for now.

LeonarddeR · 2018-02-28T20:24:43Z

@dkager: is this ready for another review by @feerrenrut?

dkager · 2018-03-03T08:47:20Z

YEs, I addressed all the reviews I am aware of.

feerrenrut

Ok, I'll go ahead and incubate this.

Merge remote-tracking branch 'origin/pr/5751' into next

lallerke1 · 2018-04-11T06:45:57Z

if it appeared in next, control panel in anniversary fall update stopped speaking

LeonarddeR · 2018-04-11T09:45:42Z

I have a commit for this, but I'm afraid @dkager doesn't allow me to push it ;)

lallerke1 · 2018-04-11T13:59:47Z

why not? He sems a good person Küldte a Windows 10 Posta Feladó: Leonard de Ruijter Elküldve: 2018. április 11., szerda 11:45 Címzett: nvaccess/nvda Másolatot kap: lallerke1; Comment Tárgy: Re: [nvaccess/nvda] Filter LTR and RTL marks from list view items(#5751) I have a commit for this, but I'm afraid @dkager doesn't allow me to push it ;) — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

…hould be windowClass

LeonarddeR · 2018-04-11T17:23:00Z

@lallerke1 commented on 11 Apr 2018, 15:59 CEST:

why not? He sems a good person

Sure, he's a tough guy! Just did not check the checkbox that allows maintainers to push commits to a pull request repository.

dkager · 2018-04-11T17:46:49Z

I had a typo that @LeonarddeR fearlessly fixed.

NVDA no longer reports (LTR and RTL marks) in Braille or per-character speech when accessing the clock in recent versions of Windows. Issue #5729

dkager added 2 commits February 13, 2016 17:52

Taskbar clock: filter out Unicode character U+200E (LTR mark) that is…

6a16a24

… included in the value on recent versions of Windows. This character is usually not defined in the liblouis braille tables. re nvaccess#5729

dkager added 4 commits February 28, 2016 14:33

Also filter out RTL marks (U+200F).

5206df5

ListItem: filter out LTR and RTL marks, similar to what is already do…

b16a291

…ne in ListItemWithoutColumnSupport.

Also filter out LTR and RTL marks in Details view columns containing …

b0798a2

…dates in Windows Explorer. For example, Date modified.

Add missing return, change use of obj.windowClassName to the local va…

19b600d

…riable className.

josephsl reviewed Apr 3, 2016
View reviewed changes

Add references to relevant issues, improve code consistency.

95c81e0

dkager changed the title ~~Improve reading of the clock in the taskbar~~ Filter RTL and RTL marks from the clock and list view items Jan 20, 2017

dkager added 2 commits April 25, 2017 16:29

Revert "Taskbar clock: discard the name attribute."

5a1ff45

This reverts commit 9293131.

Revert "Taskbar clock: filter out Unicode character U+200E (LTR mark)…

b2afa07

… that is included in the value on recent versions of Windows. This character is usually not defined in the liblouis braille tables. re nvaccess#5729" This reverts commit 6a16a24.

dkager changed the title ~~Filter RTL and RTL marks from the clock and list view items~~ Filter RTL and RTL marks from list view items Apr 25, 2017

LeonarddeR suggested changes Oct 12, 2017

View reviewed changes

Address review comments.

622ae88

dkager commented Feb 1, 2018

View reviewed changes

dkager changed the title ~~Filter RTL and RTL marks from list view items~~ Filter LTR and RTL marks from list view items Feb 1, 2018

feerrenrut reviewed Feb 5, 2018

View reviewed changes

Address review comments.

2918ca1

feerrenrut approved these changes Apr 11, 2018

View reviewed changes

feerrenrut added a commit that referenced this pull request Apr 11, 2018

Incubates #5751

eb64d16

Merge remote-tracking branch 'origin/pr/5751' into next

nvaccessAuto added the incubating label Apr 11, 2018

nvaccessAuto assigned feerrenrut Apr 11, 2018

josephsl mentioned this pull request Apr 11, 2018

NVDA version next-15023,fcbd1731 Problem with Explorer #8161

Closed

Leonard de Ruijter added 2 commits April 11, 2018 19:21

Merge remote-tracking branch 'origin/master' into i5729

3213bf1

appModules.explorer.chooseNVDAObjectOverlayClasses: windowClassName s…

b6c5963

…hould be windowClass

Add clarifying comment

f6d8bb2

michaelDCurran added a commit that referenced this pull request Apr 11, 2018

Incubates #5751

ac2d9f3

nvaccessAuto assigned michaelDCurran and unassigned feerrenrut Apr 11, 2018

feerrenrut merged commit f5bc848 into nvaccess:master May 6, 2018

nvaccessAuto removed the incubating label May 6, 2018

nvaccessAuto added this to the 2018.2 milestone May 6, 2018

feerrenrut added a commit that referenced this pull request May 6, 2018

Update changes file for PR #5751

7154caa

NVDA no longer reports (LTR and RTL marks) in Braille or per-character speech when accessing the clock in recent versions of Windows. Issue #5729

DrSooom mentioned this pull request Jun 3, 2018

German Win7: LTR and RTL characters still exist in Properties Window (explorer.exe) and in the Taskbar Clock #8361

Closed

Uh oh!

Conversation

dkager commented Feb 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkager commented Feb 24, 2016

Uh oh!

dkager commented Feb 27, 2016

Uh oh!

josephsl commented Feb 27, 2016

Uh oh!

dkager commented Feb 27, 2016

Uh oh!

dkager commented Feb 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephsl commented Apr 3, 2016

Uh oh!

dkager commented Jun 7, 2016

Uh oh!

derekriemer commented Dec 1, 2016

Uh oh!

dkager commented Dec 1, 2016

Uh oh!

dkager commented Jan 18, 2017

Uh oh!

jcsteh commented Jan 20, 2017

Uh oh!

dkager commented Jan 20, 2017

Uh oh!

dkager commented Apr 25, 2017

Uh oh!

LeonarddeR commented Jun 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkager commented Feb 1, 2018

Uh oh!

dkager left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeonarddeR commented Feb 28, 2018

Uh oh!

dkager commented Mar 3, 2018

Uh oh!

feerrenrut left a comment

Choose a reason for hiding this comment

Uh oh!

lallerke1 commented Apr 11, 2018

Uh oh!

LeonarddeR commented Apr 11, 2018

dkager commented Feb 13, 2016 •

edited

Loading