MS Excel 2010 - cannot read protected cells

[nvaccessAuto]

Reported by manish on 2015-02-26 23:49
In an excel workbook that has a sheet with protected and unprotected cells, like the one attached here, there is no way to read the content in the protected cells. I tried reading with the regular cursor, review cursor and object navigation - with the simple review mode enabled as well as disabled.
Will try to find a machine with excel 2013 and add to this description if the behavior is any different there.

[nvaccessAuto]

Attachment USA_TaxOrganizer.xlsx added by manish on 2015-02-26 23:50
Description:

[nvaccessAuto]

Attachment USA_TaxOrganizer.xlsx added by manish on 2015-02-26 23:50
Description:

[nvaccessAuto]

Comment 1 by siddhartha_iitd on 2015-06-29 10:51
The code-fix for this issue is available in branch in_t4952 at following url:
https://bitbucket.org/manish_agrawal/nvda

Please review the code.Thanks!

[nvaccessAuto]

Comment 1 by siddhartha_iitd on 2015-06-29 10:51
The code-fix for this issue is available in branch in_t4952 at following url:
https://bitbucket.org/manish_agrawal/nvda

Please review the code.Thanks!

[nvaccessAuto]

Comment 3 by mdcurran on 2015-07-22 06:53
I would suggest that rather than just speaking the cell names and values as you arrow around the cells (scriptHelper) instead you actually create an NVDA object for each cell and fire a focus event on it. That way you do not have to duplicate code for name / value etc. Also Editable should be a state then, not another spoken message.
For reading columns and rows, it would be good if it did not include blank cells.

[nvaccessAuto]

Comment 3 by mdcurran on 2015-07-22 06:53
I would suggest that rather than just speaking the cell names and values as you arrow around the cells (scriptHelper) instead you actually create an NVDA object for each cell and fire a focus event on it. That way you do not have to duplicate code for name / value etc. Also Editable should be a state then, not another spoken message.
For reading columns and rows, it would be good if it did not include blank cells.

[nvaccessAuto]

Comment 4 by siddhartha_iitd on 2015-07-31 09:29
I've made the changes suggested above. Please review the updated code in branch in_t4952_nvdaobj at https://bitbucket.org/manish_agrawal/nvda
Thanks!

[nvaccessAuto]

Comment 4 by siddhartha_iitd on 2015-07-31 09:29
I've made the changes suggested above. Please review the updated code in branch in_t4952_nvdaobj at https://bitbucket.org/manish_agrawal/nvda
Thanks!

[nvaccessAuto]

Comment 6 by mdcurran on 2015-10-08 02:08
More code review:

• rename scriptHelper to something a little more descriptive... perhaps at least navigateHelper.
• I don't understand script_activatePosition at all. It looks like code was copied from somewhere else in NVDA. It tries to create treeInterceptors on the fly etc... which just does not make any sense as we are already in a treeInterceptor.
• We have made the decision to only take the functionality specified by this ticket (i.e. moving around protected cells) but not the column and row reading. We may take that code at a later stage once the protected cells code is merged, but this should be handled by a different ticket and branch. Otherwise this gets too complicated and goes further than this ticket summary.

thanks.

[nvaccessAuto]

Comment 6 by mdcurran on 2015-10-08 02:08
More code review:

• rename scriptHelper to something a little more descriptive... perhaps at least navigateHelper.
• I don't understand script_activatePosition at all. It looks like code was copied from somewhere else in NVDA. It tries to create treeInterceptors on the fly etc... which just does not make any sense as we are already in a treeInterceptor.
• We have made the decision to only take the functionality specified by this ticket (i.e. moving around protected cells) but not the column and row reading. We may take that code at a later stage once the protected cells code is merged, but this should be handled by a different ticket and branch. Otherwise this gets too complicated and goes further than this ticket summary.

thanks.

[nvaccessAuto]

Comment 7 by siddhartha_iitd on 2015-10-08 05:40
Thanks for code review:

1. I'll rename the method.
   2)script_activatePosition is taken from script_toggleVirtualBufferPassThrough in globalCommands.py. This was suggested during our weekly call. I didn't understand much of it, can you please suggest what changes need to be made?
   3)I've logged a ticket for this. It's 5404.

[nvaccessAuto]

Comment 7 by siddhartha_iitd on 2015-10-08 05:40
Thanks for code review:

1. I'll rename the method.
   2)script_activatePosition is taken from script_toggleVirtualBufferPassThrough in globalCommands.py. This was suggested during our weekly call. I didn't understand much of it, can you please suggest what changes need to be made?
   3)I've logged a ticket for this. It's 5404.

[nvaccessAuto]

Comment 8 by jteh (in reply to comment 7) on 2015-10-08 06:07
Replying to siddhartha_iitd:

2)script_activatePosition is taken from script_toggleVirtualBufferPassThrough in globalCommands.py. This was suggested during our weekly call.

I think i mentioned setting self.passThrough in your script, but I definitely didn't mean copying that script. You will need to set self.passThrough to True and then call browseMode.reportPassThrough.

[nvaccessAuto]

Comment 8 by jteh (in reply to comment 7) on 2015-10-08 06:07
Replying to siddhartha_iitd:

2)script_activatePosition is taken from script_toggleVirtualBufferPassThrough in globalCommands.py. This was suggested during our weekly call.

I think i mentioned setting self.passThrough in your script, but I definitely didn't mean copying that script. You will need to set self.passThrough to True and then call browseMode.reportPassThrough.

[nvaccessAuto]

Comment 9 by siddhartha_iitd on 2015-10-09 11:42
Sorry for the misunderstanding. I understood it wrong. I've made the changes suggested above.
Branch name is in_t4952_nvdaobj. Please review and let me know if they are correct.

[nvaccessAuto]

Comment 9 by siddhartha_iitd on 2015-10-09 11:42
Sorry for the misunderstanding. I understood it wrong. I've made the changes suggested above.
Branch name is in_t4952_nvdaobj. Please review and let me know if they are correct.

[nvaccessAuto]

Comment 10 by Michael Curran <mick@... on 2015-10-16 01:35
In commit 1159037:
Merge branch 't4952' into next. Incubates #4952
Changes:
Added labels: incubating

[nvaccessAuto]

Comment 10 by Michael Curran <mick@... on 2015-10-16 01:35
In commit 1159037:
Merge branch 't4952' into next. Incubates #4952
Changes:
Added labels: incubating

[nvaccessAuto]

Comment 11 by mdcurran on 2015-10-16 01:44
A modified version of this code is now in NV Access branch t4952 ac8b1e5 and is now incubating in next.
It is based on code from in_t4952_nvdaobj with the following changes:

• navigationHelper was re-written to use local variables rather than storing on the treeInterceptor...
• control+arrows in browse mode now mirror what they do in normal excel spreadsheets, but of course not skipping locked cells. I.e. they now move to the edges of regions. The existing code would often move left when pressing control+rightArrow if there was no data to the right etc.
  • Added a new unlocked state, and use this instead of editable as editable was causing side-effects such as reading the value of the cell twice.
• Added the protected state to the parent table (sheet).
• Catch exceptions when arrowing to the edge of a sheet.
• Fix exceptions when pressing non-implemented quicknav keys.
• Fix typos in script_activatePosition.

[nvaccessAuto]

Comment 11 by mdcurran on 2015-10-16 01:44
A modified version of this code is now in NV Access branch t4952 ac8b1e5 and is now incubating in next.
It is based on code from in_t4952_nvdaobj with the following changes:

• navigationHelper was re-written to use local variables rather than storing on the treeInterceptor...
• control+arrows in browse mode now mirror what they do in normal excel spreadsheets, but of course not skipping locked cells. I.e. they now move to the edges of regions. The existing code would often move left when pressing control+rightArrow if there was no data to the right etc.
  • Added a new unlocked state, and use this instead of editable as editable was causing side-effects such as reading the value of the cell twice.
• Added the protected state to the parent table (sheet).
• Catch exceptions when arrowing to the edge of a sheet.
• Fix exceptions when pressing non-implemented quicknav keys.
• Fix typos in script_activatePosition.

[nvaccessAuto]

Comment 12 by James Teh <jamie@... on 2015-11-02 00:49
In commit 7e9bc71:
Excel browse mode: arrows can now move to locked cells in protected sheets. Control+arrows move to the edge of regions (just like normal Excel) but don't ignore locked cells. All unlocked cells now have a new unlocked state.

Authors: Michael Curran mick@nvaccess.org, Siddhartha gupta.siddharthasbs@gmail.com
Fixes #4952.
Changes:
Removed labels: incubating
State: closed

[nvaccessAuto]

Comment 12 by James Teh <jamie@... on 2015-11-02 00:49
In commit 7e9bc71:
Excel browse mode: arrows can now move to locked cells in protected sheets. Control+arrows move to the edge of regions (just like normal Excel) but don't ignore locked cells. All unlocked cells now have a new unlocked state.

Authors: Michael Curran mick@nvaccess.org, Siddhartha gupta.siddharthasbs@gmail.com
Fixes #4952.
Changes:
Removed labels: incubating
State: closed

[nvaccessAuto]

Comment 13 by jteh on 2015-11-02 00:50
Changes:
Milestone changed from None to 2015.4

[nvaccessAuto]

Comment 13 by jteh on 2015-11-02 00:50
Changes:
Milestone changed from None to 2015.4