Skip to content

Fix input gesture identifier normalization.#6962

Merged
jcsteh merged 3 commits into
masterfrom
i6945GestureIdOrder
Mar 29, 2017
Merged

Fix input gesture identifier normalization.#6962
jcsteh merged 3 commits into
masterfrom
i6945GestureIdOrder

Conversation

@jcsteh

@jcsteh jcsteh commented Mar 10, 2017

Copy link
Copy Markdown
Contributor

Previously, arbitrarily ordered items (e.g. multiple keys) were reordered using Python set order. However, different input ordering could incorrectly cause different output in some cases due to hash bucket conflicts. For example, dot4+space and space+dot4 incorrectly produced different output.
In addition, these items were reordered before conversion to lower case instead of after, which could also incorrectly result in different output.
Now, the entire identifier is converted to lower case and arbitrarily ordered items are then sorted by character.
The InputGesture.identifiers property has been changed so that it is no longer normalized. Instead, there is now an InputGesture.normalizedIdentifiers property which returns normalized identifiers.
This was done to make normalization changes easier to apply and to eliminate the potential for normalization errors in InputGesture subclasses.
The InputGesture.logIdentifier property is now deprecated. InputGesture.identifiers[0] should be used instead.
Finally, various InputGesture implementations have been changed to remove normalization code. In some cases, this may result in nicer reporting in input help mode.
Fixes #6945. Re #3157.

Previously, arbitrarily ordered items (e.g. multiple keys) were reordered using Python set order. However, different input ordering could incorrectly cause different output in some cases due to hash bucket conflicts. For example, dot4+space and space+dot4 incorrectly produced different output.
In addition, these items were reordered *before* conversion to lower case instead of after, which could also incorrectly result in different output.
Now, the entire identifier is converted to lower case and arbitrarily ordered items are then sorted by character.
The InputGesture.identifiers property has been changed so that it is no longer normalized. Instead, there is now an InputGesture.normalizedIdentifiers property which returns normalized identifiers.
This was done to make normalization changes easier to apply and to eliminate the potential for normalization errors in InputGesture subclasses.
The InputGesture.logIdentifier property is now deprecated. InputGesture.identifiers[0] should be used instead.
Finally, various InputGesture implementations have been changed to remove normalization code. In some cases, this may result in nicer reporting in input help mode.
@jcsteh jcsteh requested a review from feerrenrut March 10, 2017 13:02

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a little worried that there may be issues caused by localisation. Other than that a few little things I noticed and a few questions.

Comment thread source/inputCore.py
Then, any items separated by a + sign after the source prefix are considered to be of indeterminate order
and are sorted by character.
"""
identifier = identifier.lower()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this locale dependant? Will it use the system locale or the invariant locale by default?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If the string has any non-ASCII characters in it, it should be unicode, in which case .lower() will operate using the Unicode database:

>>> ua = u'\u0391'
>>> unicodedata.name(ua)
'GREEK CAPITAL LETTER ALPHA'
>>> la = ua.lower(); la
u'\u03b1'
>>> unicodedata.name(la)
'GREEK SMALL LETTER ALPHA'

Comment thread source/keyboardHandler.py
for modVk, modExt in self.generalizedModifiers:
if isNVDAModifierKey(modVk, modExt):
modTexts.add("NVDA")
modTexts.append("NVDA")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this need to be uppercase for some reason? It will get converted to lowercase right? Or is it for the log output?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It gets converted to lower case when normalised. However, we want it to be displayed in upper case in log output, since that's how we write it in documentation.

Comment thread source/touchHandler.py
ID+="%s_"%self.counterNames[min(self.tracker.actionCount,4)-1]
ID+=self.tracker.action
IDs.append("TS(%s):%s"%(self.mode,ID))
IDs.append("ts(%s):%s"%(self.mode,ID))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what does ts mean? could you add a comment to explain please.

jcsteh added a commit that referenced this pull request Mar 14, 2017
@jcsteh jcsteh merged commit 8f853ce into master Mar 29, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.2 milestone Mar 29, 2017
@jcsteh jcsteh deleted the i6945GestureIdOrder branch March 29, 2017 06:58
jcsteh added a commit that referenced this pull request Mar 29, 2017
jcsteh added a commit that referenced this pull request Jun 21, 2017
…rs; we got rid of that in #6962. Make it possible to get the display text from a braille keyboard gesture identifier (as used in the Input Gestures dialog).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants