Comment 2 by jteh on 2014-03-01 03:07
Does this occur with master or 2013.3?

Comment 3 by jteh on 2014-03-01 03:11
From the log, it looks like there is a standard Windows Edit control which also implements IAccessible2. I'm not sure why they would have done this, though. If I'm right, I don't see how this can be new in NVDA; it would always have happened. Is it possible this is due to an Eclipse update?

Comment 4 by camlorn on 2014-03-01 05:16
I think it's an Eclipse update. Q's got got an older version that works and is also on next. I assume this means we need to file with them. I can't actually open it, as I don't know enough to do so, but if you open one and link it I'll make the effort to comment.
Should NVDA handle this situation? Implementing IA2 seems like a fairly reasonable thing to do on a windows text control, if you're overriding WM_PAINT. One place this does occur is the sites list in the updating dialog, which appears to be a combobox, but provides the following developer info:

INFO - globalCommands.GlobalCommands.script_navigatorObject_devInfo (00:10:09):
Developer info for navigator object:
name: ''
role: ROLE_EDITABLETEXT
states: STATE_FOCUSABLE, STATE_FOCUSED
isFocusable: True
hasFocus: True
Python object: <NVDAObjects.Dynamic_IAccessibleEditWindowNVDAObject object at 0x05A48A50>
Python class mro: (<class 'NVDAObjects.Dynamic_IAccessibleEditWindowNVDAObject'>, <class 'NVDAObjects.IAccessible.IAccessible'>, <class 'NVDAObjects.window.edit.Edit'>, <class 'NVDAObjects.behaviors.EditableTextWithAutoSelectDetection'>, <class 'NVDAObjects.behaviors.EditableText'>, <class 'editableText.EditableText'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <type 'object'>)
description: None
location: (280, 112, 683, 15)
value: u'type or select a site'
appModule: <'appModuleHandler' (appName u'javaw', process ID 5492) at address 5a48c70>
appModule.productName: u'Java(TM) Platform SE 7 U51'
appModule.productVersion: u'7.0.510.13'
TextInfo: <class 'NVDAObjects.window.edit.EditTextInfo'>
windowHandle: 722760
windowClassName: u'Edit'
windowControlID: 1001
windowStyle: 1342177920
windowThreadID: 4356
windowText: u'type or select a site'
displayText: u'type or select a site'
IAccessibleObject: <POINTER(IAccessible) ptr=0x86e1f98 at 5a47350>
IAccessibleChildID: 0
IAccessible event parameters: windowHandle=722760, objectID=-4, childID=0
IAccessible accName: u'Work with:'
IAccessible accRole: ROLE_SYSTEM_TEXT
IAccessible accState: STATE_SYSTEM_FOCUSED, STATE_SYSTEM_FOCUSABLE, STATE_SYSTEM_VALID (1048580)
IAccessible accDescription: None
IAccessible accValue: u'type or select a site'

NVDA master does do this as well. If there is value in going back to 2013.3, I will do so.

Comment 5 by jteh on 2014-03-01 05:49
NVDA should still handle this situation. I was just curious as to why it started occurring now. I don't quite follow what IAccessibleText would give them here, but perhaps it provides additional info that they're painting visually.

That control you provided info for is just a standard Edit control. It's probably inside a combo box, but that's a standard Win32 pattern.

Comment 6 by lpintes (in reply to comment 3) on 2014-03-01 08:46
Replying to jteh:

From the log, it looks like there is a standard Windows Edit control which also implements IAccessible2. I'm not sure why they would have done this, though. If I'm right, I don't see how this can be new in NVDA; it would always have happened. Is it possible this is due to an Eclipse update?

This appears in any dialog, not only in update, if you are thinking about "software update".
My crazy theory is this:
The edit field in dialogs contains some information which can be relevant. For example, if I am creating something new and am typing the folder name, besides error sound I hear something like "the specified directory doesn't exist".
You said that the edit implements IAccessible2. Could this mean that there is a IA2 relation between something and mentioned edit field, something like "labelled_by"? If yes, this could mean that the edit must implement IAccessible2.
And I am pretty sure that the edit is an imformation, because if something didn't work for me, I focused the edit, heard error sound and did Ctrl+A, Ctrl+C. Then I red the information with NVDA+C. And I immediately knew that for example "xxx is an invalid name of something"

Comment 7 by camlorn on 2014-03-08 15:42
Is this something we need to open a ticket against Eclipse for, a problem that must be solved in the Eclipse module, or a situation that can show up anywhere that NVDA can't handle? I might take a crack at it. I'm just not sure if this is something that will show up elsewhere or at what level I should look to implement the fix.

Comment 8 by camlorn on 2014-03-09 21:28
I've done some more looking. Given that the Eclipse module is only like 5 lines, it isn't happening there. This Stackoverflow explains at least one situation that an cause the problem in the general case:
http://stackoverflow.com/questions/3003053/metaclass-multiple-inheritance-inconsistency
So it might just be flipping some base classes somewhere and seeing if NVDA crashes. I am going to go through the classes involved in the error and write down their bases, and see if I can't find the "backward" piece. I suspect that it is not this simple. I will come back here once I have tried this (but if someone beats me to it, or thinks they can, feel free).

Comment 9 by jteh on 2014-03-09 22:28
I know exactly what's happening here. As I said i comment:3, the control is a Win32 Edit control, but it also supports IAccessible2, specifically IAccessibleText. The problem is that NVDA chooses the Edit NVDAObject, which inherits from EditableText. Later, it chooses EditableText again because it detects IAccessibleText. So, it's trying to inherit from EditableText twice in two very different places in the inheritance hierarchy and Python (rightly) doesn't know what to do about it.

The question is more how to fix it. The simple answer is not to add EditableText a second time, but this isn't necessarily the right approach. Ideally, we want to use IAccessibleText, which means we we ideally want to get rid of the first instance (in this case, Win32 Edit). However, while it's Edit in this specific case, it could be many other things in other cases. So, the question is how to determine what to get rid of without killing performance.

Comment 10 by camlorn (in reply to comment 9) on 2014-03-11 01:38
Replying to jteh:

I know exactly what's happening here. As I said i comment:3, the control is a Win32 Edit control, but it also supports IAccessible2, specifically IAccessibleText. The problem is that NVDA chooses the Edit NVDAObject, which inherits from EditableText. Later, it chooses EditableText again because it detects IAccessibleText. So, it's trying to inherit from EditableText twice in two very different places in the inheritance hierarchy and Python (rightly) doesn't know what to do about it.

The question is more how to fix it. The simple answer is not to add EditableText a second time, but this isn't necessarily the right approach. Ideally, we want to use IAccessibleText, which means we we ideally want to get rid of the first instance (in this case, Win32 Edit). However, while it's Edit in this specific case, it could be many other things in other cases. So, the question is how to determine what to get rid of without killing performance.

I did not realize this had already been tracked to an MRO problem. I did read your comments. I did not realize you were talking about classes specifically and read it as general terms. The information I found on the exception via Google makes it look like a simple fix-just flip your base classes somewhere and you're done.
We could make a class-level attribute, classes_to_exclude or similar. If found and not None, check the class list and do such exclusion. Alternatively, look it up in some sort of external dict of some sort, which might open up the ability to do it on an app-specific basis. The performance penalty of this is at most the number of classes times the maximum allowed number of excluded classes, and it can probably be done against a set or an ordereddict. I want to try something before I comment further on it.
In terms of conquering performance issues, I believe Python set operations to be fast, which is a possibility, and the Blist package might provide the final needed piece (and I could argue that's a good package for add-ons anyway these days: it has replaced every other data structure I have ever used ever). Blist has a fast Orderedset-a set that respects order, with fast in operation.
I have three specific questions. The first I can probably answer myself pretty trivially by actually making the change, and seeing what I broke, but I'd appreciate input on the last two before I start seriously investigating solutions:
Will not including a base twice ever break anything?
Do we ever want to include a base twice?
And finally, what is the ideal solution to this kind of problem, in such a way that it is actually helpful in the future?

Comment 11 by camlorn on 2014-03-17 21:01
Simply making sure that no base class is ever included twice, in two different ways, did not fix the problem. I am going to continue looking into this. My implementation was incredibly straightforward: if a class appears twice, remove one of them. It changed nothing. I suspect that there is in fact a strange shape-I-can't-describe inheritance problem that Python's algorithm can't handle, or my understanding is still lacking.
Can anyone enlighten me as to why it's call__ instead of new? I'm assuming there's a reason, but am not sure if it's an important one for this issue.
Edit: Looking further, it's not the inheritance lists themselves, and it will probably work if EditableTextWithAutoSelection is included after Edit. Edit tries to override EditableTextWithAutoSelection, which is already overriding Edit, and python doesn't like it. I am reading the rest to see what actually happens in the selection process.

Comment 12 by jteh on 2014-03-18 01:22
Only have time for a brief reply just now, but note that some of the classes in the list could be subclasses, so you need to use issubclass to check. Having said that, I'm not convinced this is the best option.

Comment 13 by camlorn on 2014-03-19 01:37
Can you point me at the code that actually chooses the classes? it's either scattered all over the place, somewhere conceptually far from the exception, or in the I'm being an idiot and will find it in 5 more minutes blind spot. I am not convinced that it's the best solution either, but need to find the right place to see what's going on.
I kind of think call needs refactoring love, but am still internalizing how it works. There are some checks that I think are redundant, but if they were they wouldn't be there.
I'm going to assume you have this, but I extracted what looks like a better log. The control in the above is not the control causing the problem. This is it:

INFO - globalCommands.GlobalCommands.script_navigatorObject_devInfo (21:26:19):
Developer info for navigator object:
name: None
role: ROLE_EDITABLETEXT
states: STATE_INVISIBLE
isFocusable: False
hasFocus: False
Python object: <NVDAObjects.Dynamic_IAccessibleRichEdit50WindowNVDAObject object at 0x05686930>
Python class mro: (<class 'NVDAObjects.Dynamic_IAccessibleRichEdit50WindowNVDAObject'>, <class 'NVDAObjects.IAccessible.IAccessible'>, <class 'NVDAObjects.window.edit.RichEdit50'>, <class 'NVDAObjects.window.edit.RichEdit'>, <class 'NVDAObjects.window.edit.Edit'>, <class 'NVDAObjects.behaviors.EditableTextWithAutoSelectDetection'>, <class 'NVDAObjects.behaviors.EditableText'>, <class 'editableText.EditableText'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <type 'object'>)
description: None
location: (0, 0, 0, 0)
value: None
appModule: <'nvda' (appName 'nvda', process ID 2440) at address 53615b0>
appModule.productName: u'NVDA'
appModule.productVersion: u'next-10434,4c80d23'
TextInfo: <class 'NVDAObjects.window.edit.EditTextInfo'>
windowHandle: 395158
windowClassName: u'RICHEDIT50W'
windowControlID: 0
windowStyle: 0
windowThreadID: 2664
windowText: u''
displayText: u''
IAccessibleObject: <POINTER(IAccessible) ptr=0x8c08328 at 539bdf0>
IAccessibleChildID: 0
IAccessible event parameters: windowHandle=395158, objectID=-4, childID=0
IAccessible accName: exception: (-2147024809, 'The parameter is incorrect.', (None, None, None, 0, None))
IAccessible accRole: ROLE_SYSTEM_TEXT
IAccessible accState: STATE_SYSTEM_INVISIBLE, STATE_SYSTEM_VALID (32768)
IAccessible accDescription: None
IAccessible accValue: None

Comment 14 by jteh on 2014-03-19 01:59
The code is in quite a few places, but that's by design. The whole point is that APIs at various levels, as well as app modules and global plugins, all have the opportunity to participate in choosing classes. In this case, with reference to master, you're interested in line 114 of NVDAObjects/window/__init__.py and line 389 of NVDAObjects/IAccessible/__init__.py.

I don't think the log you just provided is the control in question. If it were, you wouldn't be able to get that log, as NVDA can't actually instantiate the problematic object at all.

Note that we can actually hack around this specifically for Eclipse very easily: just remove NVDAObjects.window.edit.Edit from the class list in the Eclipse app module's chooseNVDAObjectOverlayClasses. However, I'd prefer to come up with a more generic solution.

Actually, this makes me think we should perhaps just remove any previous editable text implementation when we detect an IAccessible2 or UIA text implementation, the argument being that the latter implementation should (in theory) be better. (Otherwise, why would it have been implemented?)

Comment 15 by camlorn on 2014-03-21 02:08
I think that Aria shows why we shouldn't have an always-on prefer X over Y. I could probably find a great number of concrete pages that misuse Aria (Jira in at least one place, Google...)and, as you have seen yourself with the Aria spec, it's "the new thing" and people do it without understanding of the actual implications. In the case of IA2, more work is provided, but whose to say that it's complete or correct? if the screen reader is too rigid to go to the alternate source, then things which may have otherwise worked stop. As I don't know of a screen reader that ignores everything when it sees IAccessible stuff, I'd hesitate to say "turn off forever"-I do not have a data point of how bad that would end up being.
I do agree on the need for a better solution. In general, I kind of wonder if the Python MRO was the best choice for this, but discussing alternate architectures that would definitely break backward compatibility with every non-nVAccess add-on is probably never going to be on the table.
One way to do it might be to remove all inheritance. This would give completely linear objects in that the bases passed in is exactly the bases that come out. Doing so could be done in a metaclass. Python's class linearization algorithm may not be appropriate in this case, as other avenues that break things really can't be easily explored. The thing that comes to mind, that we can't do, is some sort of information gathering service on a large scale: a pubsub implementation with gathering daemons, that go fill out an object without ever talking to each other, marking it complete when they're done (and thus MRO is a non-issue, and the whole fun with the metaclass is possibly avoided).
I shall read the code you've referenced. I understand how it's gathered, given that it's spelled out in call (which I can't type right without consulting Track manuals), but need to see why those classes are picked. I think a general "I am X, and I am better than Y" may be the answer, but I think that that just brings us one more step closer to the "advanced accessibility options dialog".

Comment 16 by jteh on 2014-03-21 08:34
The whole point of using classes (and thus inheritance and the mro) is that pieces of the implementation can be provided by whatever code and that higher level APIs can override lower level ones. You talk of information gathering without inheritance, but that means no overriding, which means lower levels might provide incorrect information. The core has no way to know whether that information is incorrect or just incomplete.

There is no way to use both the IAccessibleText and Edit APIs. You use one or the other. As to the problem of it breaking in rare cases, that can be easily fixed with plugins, whether bundled or external. Having said that, this is the first case of this problem we've ever seen.

Comment 17 by James Teh <jamie@... on 2014-09-11 05:46
In [01550b9]:

Hopefully fix serious problems with editable text controls in dialogs in recent versions of Eclipse.

If IAccessible2 has been implemented, don't try to use Window level classes. This prevents inheritance conflicts for mix-ins such as EditableText. If IA2 has been implemented, it should be superior anyway. Where this isn't the case, this can be worked around in app modules, etc.
Re #3872.

Comment 18 by jteh on 2014-09-11 06:19
It'd be great if anyone experiencing this could please try this try build and report whether it fixes the issue. Thanks.

Comment 19 by lpintes (in reply to comment 18) on 2014-09-11 15:03
Replying to jteh:

It'd be great if anyone experiencing this could please try this try build and report whether it fixes the issue. Thanks.

I tried and I strongly believe it is fixed. Thanks.

Comment 20 by James Teh <jamie@... on 2014-09-11 23:17
In [3802dad]:

Merge branch 't3872' into next

Incubates #3872.

Changes:
Added labels: incubating

Comment 21 by jteh on 2014-09-11 23:17
Changes:
Milestone changed from None to next

Comment 22 by James Teh <jamie@... on 2014-10-08 00:35
In [f438ab6]:

Serious problems with editable text controls in dialogs in recent versions of Eclipse have been fixed.

If IAccessible2 has been implemented, don't try to use Window level classes. This prevents inheritance conflicts for mix-ins such as EditableText. If IA2 has been implemented, it should be superior anyway. Where this isn't the case, this can be worked around in app modules, etc.
Fixes #3872.

Changes:
Removed labels: incubating
State: closed

Comment 23 by jteh on 2014-10-08 00:44
Changes:
Milestone changed from next to 2014.4