Normalize start Text pointer to support Text Cursor Indicator

[harshit7962]

Description

Using accessibility option of text cursor indicator in RichTextBox sometimes lead to crashing applications due to Invariant assert here. Adding Normalize() call in changed location moves the end to the start if this invariant assert fails and hence essentially corrects the cursor position.

Customer Impact

Accessibility fix. Allowing the usage of text cursor indicator with RichTextBox.

Regression

None

Testing

Local Build Pass
Sample Application Testing
Test Pass

Risk

Low

Microsoft Reviewers: Open in CodeFlow

[miloush]

I see change on lines 1813-15. So you get the bounding rectangle, then potentially change it and then return the previous rectangle values? Doesn't sound like the intended API experience.

I also don't see how changing the order of Normalize and GetBoundingRectangle in this method changes the invariant after exiting the method. Even if the adapter called back for a clone, it would be after Normalize before the change, so putting Normalize at the beginning of Clone should have not fixed it.

Can you share some of the investigation on what breaks the invariant or when?

[h3xds1nz]

I agree with miloush. I'm a bit puzzled. Sharing the investigation would be great to understand this properly.

So you get the bounding rectangle, then potentially change it and then return the previous rectangle values? Doesn't sound like the intended API experience.

And this exactly concerns me.

[harshit7962]

So the GetBoudingRectangles eventually leads the call to NormalizePosition method of TextPointerBase, where based on the LogicalDirection, the start pointer deviates from its expected value.

Flow:

• TextRangeAdaptor -> ITextRangeProvider.GetBoundingRectangles
• This calls GetBoundingRectangles of TextAdaptor
• which calls MoveToInsertionPosition of TextRangeAdaptor from here
• This calls the MoveToInsertionPosition of TextPointer which again calls MoveToInsertionPosition of TextPointerBase
• This is where NormalizePosition is called and our invariance breaks.

[harshit7962]

Building upon the above comment, basically the idea of the change is that GetBoundingRectangles in TextRangeAdaptor uses a start textpointer and an end textpointer, during the calculation of which it changes the start textpointer causing the invariance. After the calculation is done with, we Normalize the start pointer back to what the original value was.

How do we certain that the start is going back to its original value post Normalize:
The MoveToInsertionPosition which causes the change in GetBoundingRectangles, is also called in Normalize with LogicalDirection as reverse of what it was earlier.

cc:/ @miloush, @h3xds1nz

[h3xds1nz]

@harshit7962 sorry, somehow managed to miss this.

From a brief overview it makes sense with your explanation but I'm still not sure whether that's the correct solution; imho it shouldn't happen in the first place. Do we have a repro? I'd just like to step through.

[harshit7962]

Thanks for willing to take a look into it, for the repro, use the following xaml

<RichTextBox> 
    <FlowDocument> 
        <Paragraph> 
            <Bold>a</Bold>
            <Bold>b</Bold> 
        </Paragraph> 
   </FlowDocument> 
</RichTextBox> 

Turn on the text cursor indicator from settings. Settings -> Accessibility -> Text Cursor
Launch the application and move the indicator between a and b, it should crash the application.