search

wordpress-mobile / AztecEditor-iOS

A reusable native iOS visual HTML text editor component.
Mozilla Public License 2.0
611 stars 146 forks source link

Use Xcode 15.3 in CI #1392

Open mokagio opened 3 months ago

mokagio commented 3 months ago

Builds on top of https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1391, where I noticed that CI didn't run because we no longer have agents with Xcode 13 online.

To test: See green CI running on Xcode 15.3 image... via #1393 which marks four tests as expected failures 😞

mokagio commented 3 months ago

The test that locks is testCopyAndPasteToPlainText from TextViewTests. It didn't occur for me locally till I tried to run the test class individually.

mokagio commented 3 months ago

The test that locks is testCopyAndPasteToPlainText from TextViewTests. It didn't occur for me locally till I tried to run the test class individually.

Figured out why...

Screenshot 2024-04-16 at 19 49 34
mokagio commented 3 months ago

Ah! I also realized that @jkmassel had already worked out that issue in #1377. I had seen some code about UIPasteboard .forTesting and was surprised not to find it on my local. I then realized this branch was out of sync with develop.

AliSoftware commented 3 months ago

@mokagio I took the liberty to push a commit (https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1392/commits/665b15bd56ad4a0331c13f71045d693613486362) to fix the testPaste{Image,Video}WithoutFormatting test failures.

[EDIT]I later fixed that commit 665b15b with c67821c[/EDIT]

Indeed, apparently, even if the parent class already overrides it to return true, subclasses which override init?(coder:)/encode(with:) from their parent class need to also re-override +supportsSecureCoding.

I've added the overrides in classes that I've modified, but haven't check if maybe there are more classes that might not be covered by our unit tests around NSAttributedString archiving/pasting, but would still require it to be added. So would be worth making another pass to be sure we didn't forget any (otherwise the client app would start dismissing those attributes during pasting, while it didn't before our migration to iOS12).

AliSoftware commented 3 months ago

@mokagio Just also pushed https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1392/commits/90c0e4db1246dc4e87f004c4ec96061399447efb to fix the testCopyAndPasteToPlainText test failure on extra \n.

⚠️ That being said, while the test pass, I'm still not 100% sure why there was a \n when we were using public.plain-text before, so maybe there's more to it and it could still be worth investigating further (and add more tests to cover the cases we missed if needed)

There's also still another test failing, this time in WordPressEditor where testCaptionShortcodeIsProperlyConvertedIntoFigureTagPreservingNestedTags() seems to have an extra space in the output. I haven't looked into that one so I'll leave it to you to take a look.