Skip to content

[iOS] Bugfix/22469 ios label crash#22487

Merged
PureWeen merged 9 commits into
dotnet:mainfrom
artemvalieiev:bugfix/22469-ios-label-crash
Aug 27, 2024
Merged

[iOS] Bugfix/22469 ios label crash#22487
PureWeen merged 9 commits into
dotnet:mainfrom
artemvalieiev:bugfix/22469-ios-label-crash

Conversation

@artemvalieiev

Copy link
Copy Markdown
Contributor

Description of Change

I have found an alternative approach for calculating span regions to prevent crashes. Please try launching Issue22469 UITest with using original FormattedStringExtensions on the iPhone 15/XS simulator to reproduce the crash.

System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at System.Collections.Generic.List`1[[System.Double, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].get_Item(Int32 index)
   at Microsoft.Maui.Controls.Platform.FormattedStringExtensions.CreateSpanRects(CGRect startRect, CGRect endRect, List`1 lineHeights, List`1 multilineRects, Boolean endsWithNewLine)
   at Microsoft.Maui.Controls.Platform.FormattedStringExtensions.GetMultilinedBounds(NSRange characterRange, NSLayoutManager layoutManager, NSTextContainer textContainer, CGRect startRect, CGRect endRect, List`1 lineHeights, Boolean endsWithNewLine)
   at Microsoft.Maui.Controls.Platform.FormattedStringExtensions.RecalculateSpanPositions(UILabel control, Label element)
   at Microsoft.Maui.Controls.Label.RecalculateSpanPositions()
   at Microsoft.Maui.Controls.Label.ArrangeOverride(Rect bounds)
   at Microsoft.Maui.Controls.VisualElement.Microsoft.Maui.IView.Arrange(Rect bounds)
   at Microsoft.Maui.Layouts.GridLayoutManager.ArrangeChildren(Rect bounds)
   at Microsoft.Maui.Controls.Layout.CrossPlatformArrange(Rect bounds)
   at Microsoft.Maui.Platform.MauiView.CrossPlatformArrange(Rect bounds)
   at Microsoft.Maui.Platform.MauiView.LayoutSubviews()
   at UIKit.UIApplication.UIApplicationMain(Int32 argc, String[] argv, IntPtr principalClassName, IntPtr delegateClassName) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:line 58

In the original approach, the DefaultLineHeight is calculated and used to determine the number of text lines associated with the span, which is different from the number of lines returned by EnumerateLineFragments -> multilineRects. This causes an "argument out of range" exception in the loop at line 249, where the loop assumes that the line height count is equal to multilineRects.Count.

Instead, I am always using EnumerateLineFragments and I am trying to calculate each rect depending on line glyph indexes of this line fragment.

Video of Label controls page tests
https://github.com/dotnet/maui/assets/3391032/93f67658-71a5-4705-ab89-71004f4ad2d7

Issues Fixed

Fixes #22469

@artemvalieiev artemvalieiev requested a review from a team as a code owner May 17, 2024 12:13
@dotnet-policy-service dotnet-policy-service Bot added the community ✨ Community Contribution label May 17, 2024
@artemvalieiev

Copy link
Copy Markdown
Contributor Author

I just tried UI test from #22213 , and it works well too with my changes

@artemvalieiev

Copy link
Copy Markdown
Contributor Author

I'm guessing this would require verification from Javier @jsuarezruiz anyway

@PureWeen

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@Eilon Eilon added the area-controls-label Label, Span label May 17, 2024
@artemvalieiev

Copy link
Copy Markdown
Contributor Author

Also fixes #22480 , since using GetBoundingRect for getting text/glyph range rectangle is RTL/LTR independent

@mos379

mos379 commented Aug 13, 2024

Copy link
Copy Markdown
Contributor

Any chance this will be reviewed?

@PureWeen PureWeen requested review from tj-devel709 and removed request for PureWeen and rmarinho August 13, 2024 20:14
@tj-devel709 tj-devel709 force-pushed the bugfix/22469-ios-label-crash branch from 549ace5 to 941a1ec Compare August 16, 2024 18:53
@tj-devel709

Copy link
Copy Markdown
Member

Rebased on top of main

@tj-devel709

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@tj-devel709

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@tj-devel709

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

tj-devel709
tj-devel709 previously approved these changes Aug 21, 2024
Comment thread src/Controls/src/Core/Platform/iOS/Extensions/FormattedStringExtensions.cs Outdated
@tj-devel709

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@artemvalieiev

Copy link
Copy Markdown
Contributor Author

Sorry I am on vacation right now, thanks @tj-devel709 ❤️

@PureWeen PureWeen enabled auto-merge (squash) August 26, 2024 20:03
@tj-devel709

Copy link
Copy Markdown
Member

Sorry I am on vacation right now, thanks @tj-devel709 ❤️

No worries, enjoy your vacation! :)

@PureWeen PureWeen merged commit d93e48d into dotnet:main Aug 27, 2024
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Sep 5, 2024
@samhouts samhouts added fixed-in-9.0.0-rc.2.24503.2 and removed fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Oct 14, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[iOS] RecalculateSpanPositions causing crash

6 participants