Fix reading empty columns in Windows Explorer on Windows 7#8316
Conversation
|
Could you please provide the steps to reproduce in order to reproduce the issue you're trying to solve? |
|
Sure STR
ExpectedSize read only should be read ActualThe error sound is played and the following is recorded in the log |
|
Hi, To @dkager and @michaelDCurran: can we get this as a fast tract pull request for 2018.2 please? Thanks. |
dkager
left a comment
There was a problem hiding this comment.
Only some style comments.
| def _get_value(self): | ||
| value = super(UIProperty, self).value | ||
| return value.replace(CHAR_LTR_MARK,'').replace(CHAR_RTL_MARK,'') | ||
| if value == None: |
There was a problem hiding this comment.
While it is not really enforced, is None seems to be much more common in the source code than == None.
| return value.replace(CHAR_LTR_MARK,'').replace(CHAR_RTL_MARK,'') | ||
| if value == None: | ||
| return value | ||
| else: |
There was a problem hiding this comment.
My personal style would be to remove this else statement and just have the two return statements on consecutive lines, with the first one indented of course.
|
@dkager Thanks for your comments. I've applied your suggestions in the recent commit. |
|
|
||
| def _get_value(self): | ||
| value = super(UIProperty, self).value | ||
| if value is None: |
There was a problem hiding this comment.
Are you sure whether this is always None, i.e. why wouldn't this be an empty string?
There was a problem hiding this comment.
A never mind, I see you got a "none type object has no attribute replace" error.
There was a problem hiding this comment.
If you mean before applying my fix yes.
|
Normally, I'd advise merging this straight to master. However, since
this week is a busy week for NV Access and I'd like to have this tested
properly before the rc comes out, I will incubate this in next until
either @michaelDCurran or @feerrenrut can have a look.
|
|
I'm glad somebody reported this, I was puzzled by it only the other day, but
had not realised it was indeed what was going on!
Brian
bglists@blueyonder.co.uk
Sent via blueyonder.
Please address personal E-mail to:-
briang1@blueyonder.co.uk, putting 'Brian Gaff'
in the display name field.
----- Original Message -----
From: "Joseph Lee" <notifications@github.com>
To: "nvaccess/nvda" <nvda@noreply.github.com>
Cc: "Subscribed" <subscribed@noreply.github.com>
Sent: Monday, May 21, 2018 6:56 PM
Subject: Re: [nvaccess/nvda] Fix reading empty columns in Windows Explorer
on Windows 7 (#8316)
… Hi,
To @dkager and @michaelDCurran: can we get this as a fast tract pull
request for 2018.2 please? Thanks.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#8316 (comment)
|
Link to issue number:
None
Summary of the issue:
After Merging #5729 it was no longer possible to read empty columns Such as size for folders in Windows Explorer on Windows 7, because those columns haven't got value under this OS.
Description of how this pull request fixes the issue:
I've added conditional, which checks if value exist, and if not simply returns it without removing LTR and RTL marks.
Testing performed:
Successfully read size column for folder in Windows 7
Known issues with pull request:
None
Change log entry:
None needed, because #5729 wasn't in a release yet.