Skip to content

Commit 710abff

Browse files
kubaflomattleibow
authored andcommitted
Don’t call NSAttributedString with HTML from a background thread (#26153)
* Don’t call NSAttributedString with HTML from a background thread * Added a UITest * Refactor * add more comments and move to Controls --------- Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
1 parent 0f7b38f commit 710abff

File tree

6 files changed

+106
-9
lines changed

6 files changed

+106
-9
lines changed

src/Controls/src/Core/Label/Label.iOS.cs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,17 @@ public static void MapMaxLines(ILabelHandler handler, Label label)
3636
handler.PlatformView?.UpdateMaxLines(label);
3737
}
3838

39-
static void MapFormatting(ILabelHandler handler, Label label)
39+
internal static void MapFormatting(ILabelHandler handler, Label label)
4040
{
41-
// we need to re-apply color and font for HTML labels
42-
if (!label.HasFormattedTextSpans && label.TextType == TextType.Html)
41+
if (IsPlainText(label))
42+
{
43+
LabelHandler.MapFormatting(handler, label);
44+
}
45+
else
4346
{
4447
handler.UpdateValue(nameof(ILabel.TextColor));
4548
handler.UpdateValue(nameof(ILabel.Font));
4649
}
47-
48-
if (!IsPlainText(label))
49-
return;
50-
51-
LabelHandler.MapFormatting(handler, label);
5250
}
5351

5452
void RecalculateSpanPositions(Size size)

src/Controls/src/Core/Platform/iOS/Extensions/LabelExtensions.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,22 @@ public static void UpdateText(this UILabel platformLabel, Label label)
1414
switch (label.TextType)
1515
{
1616
case TextType.Html:
17-
platformLabel.UpdateTextHtml(label);
17+
// NOTE: Setting HTML text this will crash with some sort of consistency error.
18+
// https://github.com/dotnet/maui/issues/25946
19+
// Here we have to dispatch back the the main queue to avoid the crash.
20+
// This is observed with CarouselView 1 but not with 2, so hopefully this
21+
// will be just disappear once we switch.
22+
CoreFoundation.DispatchQueue.MainQueue.DispatchAsync(() =>
23+
{
24+
platformLabel.UpdateTextHtml(label);
25+
26+
if (label.Handler is LabelHandler labelHandler)
27+
Label.MapFormatting(labelHandler, label);
28+
29+
// NOTE: Because we are updating text outside the normal layout
30+
// pass, we need to invalidate the measure for the next pass.
31+
label.InvalidateMeasure();
32+
});
1833
break;
1934

2035
default:
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
3+
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
4+
x:Class="Maui.Controls.Sample.Issues.Issue25946">
5+
<Grid RowDefinitions="*,Auto">
6+
<CarouselView x:Name="collectionView">
7+
<CarouselView.ItemsSource>
8+
<x:Array Type="{x:Type x:String}">
9+
<x:String>Item1</x:String>
10+
<x:String>Item2</x:String>
11+
<x:String>Item3</x:String>
12+
</x:Array>
13+
</CarouselView.ItemsSource>
14+
<CarouselView.ItemTemplate>
15+
<DataTemplate>
16+
<ContentView>
17+
<VerticalStackLayout HorizontalOptions="Center"
18+
WidthRequest="200"
19+
HeightRequest="200"
20+
Background="Green">
21+
<Label Text="{Binding .}"/>
22+
<Label Text="{Binding .}"
23+
TextType="Html"/>
24+
</VerticalStackLayout>
25+
</ContentView>
26+
</DataTemplate>
27+
</CarouselView.ItemTemplate>
28+
</CarouselView>
29+
<Button Grid.Row="1"
30+
AutomationId="btnScrollToLastItem"
31+
Clicked="ButtonClicked"
32+
Text="Scroll to last Item"/>
33+
</Grid>
34+
</ContentPage>
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
namespace Maui.Controls.Sample.Issues
2+
{
3+
[Issue(IssueTracker.Github, 25946, "App crashes on iOS 18 when placing html label in carousel view with > 2 elements", PlatformAffected.iOS)]
4+
public partial class Issue25946 : ContentPage
5+
{
6+
public Issue25946()
7+
{
8+
InitializeComponent();
9+
}
10+
11+
void ButtonClicked(object sender, EventArgs e)
12+
{
13+
collectionView.ScrollTo(1, animate: true);
14+
}
15+
}
16+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using NUnit.Framework;
2+
using UITest.Appium;
3+
using UITest.Core;
4+
5+
namespace Microsoft.Maui.TestCases.Tests.Issues
6+
{
7+
public class Issue25946 : _IssuesUITest
8+
{
9+
public Issue25946(TestDevice testDevice) : base(testDevice)
10+
{
11+
}
12+
13+
public override string Issue => "App crashes on iOS 18 when placing html label in carousel view with > 2 elements";
14+
15+
[Test]
16+
[Category(UITestCategories.CarouselView)]
17+
[Category(UITestCategories.Label)]
18+
public void AppShouldNotCrash()
19+
{
20+
App.WaitForElement("btnScrollToLastItem");
21+
App.Click("btnScrollToLastItem");
22+
23+
App.WaitForElement("Item2");
24+
}
25+
}
26+
}

src/Core/src/Platform/iOS/LabelExtensions.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ internal static void UpdateTextHtml(this UILabel platformLabel, ILabel label)
8888
};
8989

9090
NSError nsError = new();
91+
92+
// NOTE: Sometimes this will crash with some sort of consistency error.
93+
// https://github.com/dotnet/maui/issues/25946
94+
// The caller should ensure that this extension is dispatched. We cannot
95+
// do it here as we need to re-apply the formatting and we cannot call
96+
// into Controls from Core.
97+
// This is observed with CarouselView 1 but not with 2, so hopefully this
98+
// will be just disappear once we switch.
9199
#pragma warning disable CS8601
92100
platformLabel.AttributedText = new NSAttributedString(text, attr, ref nsError);
93101
#pragma warning restore CS8601

0 commit comments

Comments
 (0)