Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

office 2010| selected symbol in insert symbols dialog is not read in word and excel #3538

Closed
nvaccessAuto opened this Issue Sep 21, 2013 · 20 comments

Comments

Projects
None yet
1 participant

Reported by manish on 2013-09-21 10:47
NVDA does not read the symbols as a user selects them using the arrow keys in the insert|symbols dialog box in office applications in Word 2010 and Excel 2010.

Comment 1 by manish on 2013-09-21 11:18
The problem here is that the symbols displayed are graphics/images contained in some kind of a non-standard list box.
As a user selects individual symbols using the arrow keys, no event like selection, state, value change etc. is raised by this list box (as seen using accevent.exe).

The graphic object itself does not expose any of the iaccessible properties (name, value, description, display text etc) with any useful information.
When a graphic is selected, its parent object, an object of type sdm dialog, exposes a description property that has the unicode page description of the selected graphic in the following form:
description: u'Recently used symbols:\nCopyright Sign\nShortcut key:\nAlt+0169'

Within the symbols dialog, there is a rich edit field that displays the hex unicode value of the selected character. Every time a different character is selected, the value displayed in this richedit box changes and this box raises a value changed event. The value property of the field is the hex value of the unicode character.
Using the above information, there are 2 ways of fixing this issue:

  1. on the value changed event of the richedit field, speak the character created using the value of the richedit field.
    speech.speakText(unichr(int(self.value, 16)))
    Now when the user selects each item using the arrow keys, NVDA is able to speak the current item.
  2. When the graphic object gets focus, assign it a class that contains gesture bindings for the arrow keys and home and end keys. On receiving these gestures, speak the description of the parent object.
    speech.speakText(self.parent.description) #some extra code to extract only the unicode character description.

In approach 1, we are using an event raised by and the value of a control other than the one the user is directly manipulating. in approach 2, the solution is independent of any other control but we are doing more work in the form of gesture bindings etc.
Which approach is better?

Also, for the string to be actually spoken in approach 1, we can also use
speech.speakText(api.getFocusObject().parent.description)
to use the unicode page description exposed by the dialog itself instead of relying on NVDA's unicode to character mapping. On using the unichr(int) approach, NVDA does not have description mappings for a large number of the symbols in this list and so, it ends up saying "letter 1264" r something for each selection.

To identify the richedit field and assign it a special class that handles the value change event, can we use in IAccessible.init.findOverlayClasses:
if ("RichEdit20" in windowClassName and api.getFocusObject().role==controlTypes.ROLE_GRAPHIC):
parentWindow=winUser.getAncestor(self.windowHandle,winUser.GA_PARENT)
if (winUser.getClassName(parentWindow).startswith('bosa_sdm') and winUser.isDescendantWindow(api.getFocusObject().parent.windowHandle, parentWindow)):
from .msOffice import RichEditGraphic
clsList.append(RichEditGraphic)

Comment 2 by manish on 2013-09-22 07:43
I have created a fix with both approaches on different branches at:
https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

The keyboard binding approach is at:
branch t3538-kb

and the richedit field value change approach is at:
branch t3538

Please review and pick whichever looks more correct.

Comment 3 by manish on 2013-09-22 07:48
In word 2010, the symbols dialog box in which this problem is seen is displayed by going to insert|symbols|more symbols. I forgot to mention this in the bug description and don't see an option to edit it now.

Comment 5 by manish on 2013-10-03 05:10
Tested the fix with office 2007 and 2013 as well. The t3538-kb branch is updated with a small addition required for office 2013. Now it works nicely with word and excel 2007, 2010, and 2013 symbols dialog boxes.

Comment 6 by mdcurran (in reply to comment 5) on 2013-10-03 05:18
Replying to manish:
Which branch did you test with? I'm really not too bothered which approach we take. so I'll just for now go with the one you are most happy with and the one you tested on those other versions.

Comment 7 by manish (in reply to comment 6) on 2013-10-04 04:51
I have tested branch t3538-kb with office 2007, 2010, and 2013. available at:
https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git
This is the approach that has our key bindings and works better in my opinion.
Replying to mdcurran:

Replying to manish:

Which branch did you test with? I'm really not too bothered which approach we take. so I'll just for now go with the one you are most happy with and the one you tested on those other versions.

Comment 8 by mdcurran (in reply to comment 7) on 2013-10-04 06:08
Replying to manish:
A question and a suggestion:

  • Do you know why you can't use __gestures rather than binding at runtime in initOverlayClass? I'm sure you have a good reason but just would like to understand.
    • Consider perhaps defining a name property on sdmSymbol which just fetches parent.description, and then change the scripts to execute self.event_nameChange() so that later reviewing of the graphic reports the symbol... that should also allow braille to update as well I think.
      If that change sounds okay to you, but you're busy with other things, I'm happy to take your branch as is and make that change myself and get it into next.

Comment 9 by manish (in reply to comment 8) on 2013-10-05 05:04
Thanks for your comments. Have incorporated the changes in the t3538-kb branch.

There was no good reason not to use __gestures. I just saw both styles used in existing code and thought of avoiding the repetition of the script name, which is the same for all gestures.
Replying to mdcurran:

Replying to manish:

A question and a suggestion:

  • Do you know why you can't use __gestures rather than binding at runtime in initOverlayClass? I'm sure you have a good reason but just would like to understand.
    • Consider perhaps defining a name property on sdmSymbol which just fetches parent.description, and then change the scripts to execute self.event_nameChange() so that later reviewing of the graphic reports the symbol... that should also allow braille to update as well I think.

If that change sounds okay to you, but you're busy with other things, I'm happy to take your branch as is and make that change myself and get it into next.

Comment 10 by Michael Curran <mick@... on 2013-10-06 22:59
In [16385e2]:

Merge branch 't3538' into next. Incubates #3538

Changes:
Added labels: incubating

Comment 11 by mdcurran on 2013-10-06 23:07
t3538-kb changes have been placed in an NV Access branch (t3538) which has now been merged to next for incubation.
Thanks for the code contribution. However,
I had to do some tricky rebasing to use the changes as it seems that you had been merging next into your t3538-kb. Can I advise that you try not doing that in future. Rather you should only merge master. The next branch is a temporary merge of multiple feature branches and could be reverted or rebased at any time. If you really need a next with your branch in it, consider making your own copy of next and merging your feature branch.

Comment 12 by manish (in reply to comment 11) on 2013-10-07 04:44
Replying to mdcurran:
I don't follow the comment about the rebasing.
I merged next into the t3538-kb branch to make it easier for you to merge it back into next -- my intention was to see if there will be any conflicts when this feature branch was merged into next and resolve them, if required.
Wouldn't you use the following steps to merge a feature branch:

  1. create the feature branch from master
  2. make your changes.
  3. on feature branch git merge next
  4. resolve any conflicts on feature branch.
  5. on next branch, git merge feature-branch -- this will not give any conflicts since those have already been resolved.

The deviation I did from above for t3538-kb was that I created it from next, which I'll avoid in the future.
I may be totally wrong since this is the first project I am using git on. Will appreciate any guidance for future work.

-Manish

t3538-kb changes have been placed in an NV Access branch (t3538) which has now been merged to next for incubation.

Thanks for the code contribution. However,

I had to do some tricky rebasing to use the changes as it seems that you had been merging next into your t3538-kb. Can I advise that you try not doing that in future. Rather you should only merge master. The next branch is a temporary merge of multiple feature branches and could be reverted or rebased at any time. If you really need a next with your branch in it, consider making your own copy of next and merging your feature branch.

Comment 13 by mdcurran (in reply to comment 12) on 2013-10-07 04:59
Replying to manish:
Its true it could be easier to fix merge conflicts by merging next into the feature branch first, but by doing that you then make it impossible, or at least very difficult to merge the feature branch into master when its time to do so. As, now your feature branch contains all of next in its history, but next should never get merged into master.
I appreciate the extra work to try to make it easier for us, but its best that people who work on feature branches just work on the feature, always based off of master. Next is simply a collection of possible features, managed by NV Access. It shouldn't really be thought of as an actual branch in its own right.
Perhaps Jamie can provide some further details if this wasn't too clear.

Comment 14 by jteh on 2013-11-01 01:50
We're deferring merging this into master for now, so it won''t make it into 2013.3. This is due to a few issues:

  1. This code extracts text from the dialog description. The dialog description is actually calculated by NVDA based on other objects. Aside from this being inefficient, this means there is a more direct way of fetching the symbol using some descendant of the dialog.
  2. According to Mick, there seem to be two focusable graphics in that dialog, at least in some cases. We don't understand how this works as far as selecting a symbol is concerned.
  3. This code is being used in dialogs where it probably shouldn't. We need to restrict its use if possible.

Sorry i didn't get a chance to look at this properly sooner.

Comment 15 by manish (in reply to comment 14) on 2013-11-01 17:41
I will work on points 1 and 3.
I don't think I understand point 2 .
A graphic can be selected from 2 lists in the same dialog. The first list is a list displaying all graphics while the second one displays only a few recently used graphics. The description property correctly follows the item selected in the list that was last focussed.
If you could elaborate the problem in point 2 a bit more, it will help me look for a solution there as well.

Thanks,
Manish
Replying to jteh:

We're deferring merging this into master for now, so it won''t make it into 2013.3. This is due to a few issues:

  1. This code extracts text from the dialog description. The dialog description is actually calculated by NVDA based on other objects. Aside from this being inefficient, this means there is a more direct way of fetching the symbol using some descendant of the dialog.
  2. According to Mick, there seem to be two focusable graphics in that dialog, at least in some cases. We don't understand how this works as far as selecting a symbol is concerned.
  3. This code is being used in dialogs where it probably shouldn't. We need to restrict its use if possible.

Sorry i didn't get a chance to look at this properly sooner.

Comment 16 by jteh (in reply to comment 15) on 2013-11-01 22:16
Replying to manish:

I will work on points 1 and 3.

Thanks. I recognise that point 3 might not be fixable, but give it a shot. Point 1 definitely needs to be fixed.

I don't think I understand point 2 .

A graphic can be selected from 2 lists in the same dialog. The first list is a list displaying all graphics while the second one displays only a few recently used graphics. The description property correctly follows the item selected in the list that was last focussed.

Ah., Thanks for explaining. It was probably just a misunderstanding on our part; we only had time to look briefly. One question, though: many users seem to select something from a list, then tab to the button to close the dialog, rather than just pressing enter on the list item. I assume that if you do that in this case and you're on the first list, you will lose your selection, since the first item in the second list will get focus and therefore get selected? It isn't our responsibility to change this, but I'm curious.

Comment 17 by Michael Curran <mick@... on 2013-11-27 01:54
In [9a6bdc6]:

Merge branch 't3538' into next. Incubates #3538

Comment 18 by Michael Curran <mick@... on 2013-12-12 04:19
In [846b8c3]:

Merge branch 't3538'. Fixes #3538

Changes:
Removed labels: incubating
State: closed

Comment 19 by mdcurran on 2013-12-12 04:21
Changes:
Milestone changed from None to 2014.1

Comment 20 by ondrosik on 2014-02-05 07:29
Is it supposed to work in older ms office like word 2003? Just asking, because here it doesn't read selected symbols. So if this is only for 2007 or higher, this probably needs to be specified in changelog.

Comment 21 by blindbhavya on 2014-10-03 13:31
Hi.
This shouldn't require code review still.
Probably the keywords should be removed.

@nvaccessAuto nvaccessAuto added this to the 2014.1 milestone Nov 10, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment