Assertion failed: If we select more than one this way, we have problems

[zepumph]

I came across this while trying to implement accessibleName setter for #795. @jonathanolson said he would take a look at it. Thank you! Let me know if I can help.

[zepumph]

Marking as high priority because it is blocking a high priority issue. @jonathanolson let me know if there is anything I can do to help.

[zepumph]

I found another assertion that triggered while working on AccessibilityTest.js. If I add the following line to the end of the test: `tagName/innerContent options'

rootNode.addChild( new Node( { tagName: 'div'}));

it fails out with another assertion in AccessibleInstance.sortChildren:449 (as of this writing)
assert && assert( targetChildren.length === this.children.length );

It also failed out when the tag was 'input'.

This may be similar to the other assertion above, so I thought I would add it in here. I'm happy to pair on this too so that next time I might not need to take your time to debug this a11y scenery internal stuff.

[jonathanolson]

Looks potentially related. I'm still working at understanding the cause of the failure here, but the root cause is 2 "identical" accessible instances are created where only 1 should be created. I have checked in the way to reproduce in the playground, and with the logging output, it logs:

accessibleTest()

[AccessibilityTree] accessibleContentChange n#2: had:false, has:true
| [AccessibleInstance] removeAllChildren on 1#{Empty Trail}
| [AccessibilityTree] createTree [Trail  2] parent:1#{Empty Trail}
| | [AccessibilityTree] effectiveChildren: []
| | [AccessibleInstance] Initialized 2#{[Trail  2]}
| | [AccessibleInstance] addConsecutiveInstances on 2#{[Trail  2]} with: 
| [AccessibleInstance] addConsecutiveInstances on 1#{Empty Trail} with: 2#{[Trail  2]}
[AccessibleDisplaysInfo] onSummaryChange n#2 wasA11y:false, isA11y:true
| [AccessibleDisplaysInfo] addAllAccessibleDisplays n#2
| | [AccessibleDisplaysInfo] addAccessibleDisplays n#2 numDisplays:1
[AccessibilityTree] accessibleContentChange n#11: had:false, has:true
[AccessibleDisplaysInfo] onSummaryChange n#11 wasA11y:false, isA11y:true
| [AccessibleDisplaysInfo] addAllAccessibleDisplays n#11
| | [AccessibleDisplaysInfo] addAccessibleDisplays n#11 numDisplays:0
[Accessibility] onAccessibleAddChild n#11 (parent:n#2)
| [AccessibleDisplaysInfo] onAddChild n#11 (parent:n#2)
| | [AccessibleDisplaysInfo] addAccessibleDisplays n#11 numDisplays:1
| [AccessibilityTree] addChild parent:n#2, child:n#11
| | [AccessibilityTree] addTree parent:n#2, child:n#11
| | | [AccessibilityTree] trail: [Trail  2] full:[Trail  2] for 2#{[Trail  2]} root:false
| | | | [AccessibilityTree] createTree [Trail 0 2-11] parent:2#{[Trail  2]}
| | | | | [AccessibilityTree] effectiveChildren: []
| | | | | [AccessibleInstance] Initialized 3#{[Trail 0 2-11]}
| | | | | [AccessibleInstance] addConsecutiveInstances on 3#{[Trail 0 2-11]} with: 
| | | | [AccessibleInstance] addConsecutiveInstances on 2#{[Trail  2]} with: 3#{[Trail 0 2-11]}
[AccessibilityTree] accessibleContentChange n#12: had:false, has:true
[AccessibleDisplaysInfo] onSummaryChange n#12 wasA11y:false, isA11y:true
| [AccessibleDisplaysInfo] addAllAccessibleDisplays n#12
| | [AccessibleDisplaysInfo] addAccessibleDisplays n#12 numDisplays:0
[Accessibility] onAccessibleAddChild n#12 (parent:n#11)
| [AccessibleDisplaysInfo] onAddChild n#12 (parent:n#11)
| | [AccessibleDisplaysInfo] addAccessibleDisplays n#12 numDisplays:1
| [AccessibilityTree] addChild parent:n#11, child:n#12
| | [AccessibilityTree] addTree parent:n#11, child:n#12
| | | [AccessibilityTree] trail: [Trail  11] full:[Trail 0 2-11] for 3#{[Trail 0 2-11]} root:false
| | | | [AccessibilityTree] createTree [Trail 0.0 2-11-12] parent:3#{[Trail 0 2-11]}
| | | | | [AccessibilityTree] effectiveChildren: []
| | | | | [AccessibilityTree] accessibleContentChange n#12: had:true, has:true
| | | | | | [AccessibilityTree] removeTree parent:n#11, child:n#12
| | | | | | | [AccessibleInstance] removeInstancesForTrail on 3#{[Trail 0 2-11]} with trail [Trail 0.0 2-11-12]
| | | | | | [AccessibilityTree] addTree parent:n#11, child:n#12
| | | | | | | [AccessibilityTree] trail: [Trail  11] full:[Trail 0 2-11] for 3#{[Trail 0 2-11]} root:false
| | | | | | | | [AccessibilityTree] createTree [Trail 0.0 2-11-12] parent:3#{[Trail 0 2-11]}
| | | | | | | | | [AccessibilityTree] effectiveChildren: []
| | | | | | | | | [AccessibleInstance] Initialized 5#{[Trail 0.0 2-11-12]}
| | | | | | | | | [AccessibleInstance] addConsecutiveInstances on 5#{[Trail 0.0 2-11-12]} with: 
| | | | | | | | [AccessibleInstance] addConsecutiveInstances on 3#{[Trail 0 2-11]} with: 5#{[Trail 0.0 2-11-12]}
| | | | | [AccessibilityTree] accessibleContentChange n#12: had:true, has:true
| | | | | | [AccessibilityTree] removeTree parent:n#11, child:n#12
| | | | | | | [AccessibleInstance] removeInstancesForTrail on 3#{[Trail 0 2-11]} with trail [Trail 0.0 2-11-12]
| | | | | | | | [AccessibleInstance] Disposing 5#{[Trail 0.0 2-11-12]}
| | | | | | [AccessibilityTree] addTree parent:n#11, child:n#12
| | | | | | | [AccessibilityTree] trail: [Trail  11] full:[Trail 0 2-11] for 3#{[Trail 0 2-11]} root:false
| | | | | | | | [AccessibilityTree] createTree [Trail 0.0 2-11-12] parent:3#{[Trail 0 2-11]}
| | | | | | | | | [AccessibilityTree] effectiveChildren: []
| | | | | | | | | [AccessibleInstance] Initialized 5#{[Trail 0.0 2-11-12]}
| | | | | | | | | [AccessibleInstance] addConsecutiveInstances on 5#{[Trail 0.0 2-11-12]} with: 
| | | | | | | | [AccessibleInstance] addConsecutiveInstances on 3#{[Trail 0 2-11]} with: 5#{[Trail 0.0 2-11-12]}
| | | | | [AccessibleInstance] Initialized 4#{[Trail 0.0 2-11-12]}
| | | | | [AccessibleInstance] addConsecutiveInstances on 4#{[Trail 0.0 2-11-12]} with: 
| | | | [AccessibleInstance] addConsecutiveInstances on 3#{[Trail 0 2-11]} with: 4#{[Trail 0.0 2-11-12]}

assert.js:21 Assertion failed: If we select more than one this way, we have problems

[zepumph]

@jonathanolson hunted down this bug and shared it with me today. Basically it has to do with reentrance. I was calling the accessibleName setter in invalidateAccessibleContent, which was setting labelContent and potentially calling invalidateAccessibleContent() again. In so many words, basically the view (AccessiblePeer) was calling over to SET the model (Accessibility.js), which was in response triggering a redraw of the view (reentrance). The problem will be fixed as part of #814 and #795. Closing. Thanks @jonathanolson for all the good work.