Skip to content

ui.browseableMessage(): Fix an Unicode problem when NVDA is installed in a path containing a non-ASCII character#7474

Merged
michaelDCurran merged 1 commit into
nvaccess:masterfrom
AAClause:ui_browseableMessage
Nov 23, 2017
Merged

ui.browseableMessage(): Fix an Unicode problem when NVDA is installed in a path containing a non-ASCII character#7474
michaelDCurran merged 1 commit into
nvaccess:masterfrom
AAClause:ui_browseableMessage

Conversation

@AAClause

@AAClause AAClause commented Aug 4, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

If NVDA is installed in a path containing a non-ASCII character (eg: éè), ui.browseableMessage() fails.

>>> import ui
>>> ui.browseableMessage('test')
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "ui.pyo", line 49, in browseableMessage
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe9 in position 7: ordinal not in range(128)

@AAClause
ui.browseableMessage(): Fix an Unicode problem when NVDA is install…
080dcce 
…ed in a path containing a non-ASCII character.
@LeonarddeR

LeonarddeR commented Aug 4, 2017

Copy link
Copy Markdown
Collaborator

Looks great and sensible, only one line of changed code, so I'd suggest merging this directly into master

@derekriemer

derekriemer commented Aug 11, 2017

Copy link
Copy Markdown
Collaborator

This needs code review and should be relatively easy.

@LeonarddeR LeonarddeR requested a review from feerrenrut October 12, 2017 18:55
LeonarddeR
LeonarddeR approved these changes Oct 12, 2017
View reviewed changes

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a one liner

@feerrenrut

feerrenrut commented Oct 16, 2017

Copy link
Copy Markdown
Contributor

Overall this looks good. Could you please describe the testing that has been done. In particular I want to see that the following has been tested:

Test the following with and without non-ASCIIcharacters:

  • Standard installation
  • Portable installation
  • Running from launcher (inside a folder with / without non-ASCII characters)

@AAClause

AAClause commented Nov 1, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hello,
I've just tested. I generated a build with this change.
Everything works for me! :)

1/ I have moved the launcher archive in a folder with non-ASCII characters (C:\André).
2/ I have run the launcher archive and I launched the Python Console.
I typed this input: import ui; ui.browseableMessage(u'éèçtest').
3/ I created a portable installation (in C:\André\çéèâ).
4/ I started the portable copy and I reiterated with the Python Console and with an add-on.

@feerrenrut

feerrenrut commented Nov 1, 2017

Copy link
Copy Markdown
Contributor

Great, I'll incubate this change.

feerrenrut added a commit that referenced this pull request Nov 1, 2017
@feerrenrut
Incubate #7474
1014f6d 
Merge remote-tracking branch 'origin/pr/7474' into next
@michaelDCurran michaelDCurran merged commit 5b475a8 into nvaccess:master Nov 23, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.4 milestone Nov 23, 2017
@AAClause AAClause deleted the ui_browseableMessage branch December 5, 2017 15:09
@feerrenrut feerrenrut removed their request for review February 6, 2018 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@LeonarddeR LeonarddeR LeonarddeR approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@feerrenrut feerrenrut

Labels

quick fix

Projects

None yet

Milestone

2017.4

Development

Successfully merging this pull request may close these issues.

6 participants

@AAClause @LeonarddeR @derekriemer @feerrenrut @michaelDCurran @nvaccessAuto