From d066ced2c3d95cb0c1b2b0295707d4ffefcfbe2e Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Mon, 27 May 2024 17:42:51 -0700 Subject: [PATCH] Changed linkification on paste to match URL parsing within the linkification reaction (Resolves #2031) (#2045) --- .../common_editor_operations.dart | 19 +- .../default_document_editor_reactions.dart | 23 +- .../super_editor/text_entry/links_test.dart | 230 +++++++++++++----- 3 files changed, 195 insertions(+), 77 deletions(-) diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index c3dc2d724..8c5374701 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -4,11 +4,13 @@ import 'dart:ui'; import 'package:attributed_text/attributed_text.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; +import 'package:linkify/linkify.dart'; import 'package:super_editor/src/core/document.dart'; import 'package:super_editor/src/core/document_composer.dart'; import 'package:super_editor/src/core/document_layout.dart'; import 'package:super_editor/src/core/document_selection.dart'; import 'package:super_editor/src/core/editor.dart'; +import 'package:super_editor/src/default_editor/default_document_editor_reactions.dart'; import 'package:super_editor/src/default_editor/list_items.dart'; import 'package:super_editor/src/default_editor/paragraph.dart'; import 'package:super_editor/src/default_editor/selection_upstream_downstream.dart'; @@ -2381,11 +2383,18 @@ class PasteEditorCommand implements EditCommand { for (final wordBoundary in wordBoundaries) { final word = wordBoundary.textInside(pastedText); - final link = Uri.tryParse(word); - if (link != null && link.hasScheme && link.hasAuthority) { - // Valid url. Apply [LinkAttribution] to the url - final linkAttribution = LinkAttribution.fromUri(link); + final extractedLinks = linkify( + word, + options: const LinkifyOptions( + humanize: false, + looseUrl: true, + ), + ); + final int linkCount = extractedLinks.fold(0, (value, element) => element is UrlElement ? value + 1 : value); + if (linkCount == 1) { + // The word is a single URL. Linkify it. + final uri = parseLink(word); final startOffset = wordBoundary.start; // -1 because TextPosition's offset indexes the character after the @@ -2394,7 +2403,7 @@ class PasteEditorCommand implements EditCommand { // Add link attribution. linkAttributionSpans.addAttribution( - newAttribution: linkAttribution, + newAttribution: LinkAttribution.fromUri(uri), start: startOffset, end: endOffset, ); diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index 51cb29247..a04d6d5f0 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -634,7 +634,7 @@ class LinkifyReaction implements EditReaction { final int linkCount = extractedLinks.fold(0, (value, element) => element is UrlElement ? value + 1 : value); if (linkCount == 1) { // The word is a single URL. Linkify it. - final uri = _parseLink(word); + final uri = parseLink(word); text.addAttribution( LinkAttribution.fromUri(uri), @@ -799,21 +799,24 @@ class LinkifyReaction implements EditReaction { if (updatePolicy == LinkUpdatePolicy.update) { changedNodeText.addAttribution( LinkAttribution.fromUri( - _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), + parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), ), rangeToUpdate, ); } } +} - /// Parses the [text] as [Uri], prepending "https://" if it doesn't start - /// with "http://" or "https://". - Uri _parseLink(String text) { - final uri = text.startsWith("http://") || text.startsWith("https://") // - ? Uri.parse(text) - : Uri.parse("https://$text"); - return uri; - } +/// Parses the [text] as [Uri], prepending "https://" if it doesn't start +/// with "http://" or "https://". +// TODO: Make this private again. It was private, but we have some split linkification between the reaction +// and the paste behavior in common_editor_operations. Once we create a way for reactions to identify +// paste behaviors, move the paste linkification into the linkify reaction and make this private again. +Uri parseLink(String text) { + final uri = text.startsWith("http://") || text.startsWith("https://") // + ? Uri.parse(text) + : Uri.parse("https://$text"); + return uri; } /// Configuration for the action that should happen when a text containing diff --git a/super_editor/test/super_editor/text_entry/links_test.dart b/super_editor/test/super_editor/text_entry/links_test.dart index 51a4aff47..2c3bc6e9d 100644 --- a/super_editor/test/super_editor/text_entry/links_test.dart +++ b/super_editor/test/super_editor/text_entry/links_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_robots/flutter_test_robots.dart'; @@ -9,7 +10,7 @@ import '../supereditor_test_tools.dart'; void main() { group('SuperEditor link editing >', () { - group('recognizes a URL and converts it to a link', () { + group('recognizes a URL with https and www and converts it to a link', () { testWidgetsOnAllPlatforms('when typing', (tester) async { await tester // .createDocument() @@ -1082,7 +1083,7 @@ void main() { }); }); - testWidgetsOnAllPlatforms('recognizes a second URL when typing and converts it to a link', (tester) async { + testWidgetsOnAllPlatforms('inserts https scheme if it is missing', (tester) async { await tester // .createDocument() .withSingleEmptyParagraph() @@ -1092,35 +1093,30 @@ void main() { // Place the caret at the beginning of the empty document. await tester.placeCaretInParagraph("1", 0); - // Type text with two URLs - await tester.typeImeText("https://www.google.com and https://flutter.dev "); + // Type a URL. It shouldn't linkify until we add a space. + await tester.typeImeText("www.google.com"); - // Ensure both URLs are linkified with the correct URLs. - final text = SuperEditorInspector.findTextInComponent("1"); + // Type a space, to cause a linkify reaction. + await tester.typeImeText(" "); - expect(text.text, "https://www.google.com and https://flutter.dev "); - expect( - text.hasAttributionsThroughout( - attributions: { - LinkAttribution.fromUri(Uri.parse("https://www.google.com")), - }, - range: const SpanRange(0, 21), - ), - isTrue, - ); + // Ensure it's linkified with a URL schema. + var text = SuperEditorInspector.findTextInComponent("1"); + text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "www.google.com "); expect( - text.hasAttributionsThroughout( - attributions: { - LinkAttribution.fromUri(Uri.parse("https://flutter.dev")), - }, - range: const SpanRange(27, 45), - ), - isTrue, + text.getAttributionSpansByFilter((a) => a is LinkAttribution), + { + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://www.google.com")), + start: 0, + end: 13, + ), + }, ); }); - testWidgetsOnAllPlatforms('recognizes a URL without www and converts it to a link', (tester) async { + testWidgetsOnAllPlatforms('recognizes a URL without https and www and converts it to a link', (tester) async { await tester // .createDocument() .withSingleEmptyParagraph() @@ -1153,17 +1149,18 @@ void main() { expect(text.text, "google.com "); expect( - text.hasAttributionsThroughout( - attributions: { - LinkAttribution.fromUri(Uri.parse("https://google.com")), - }, - range: SpanRange(0, text.length - 2), - ), - isTrue, + text.getAttributionSpansByFilter((a) => a is LinkAttribution), + { + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://google.com")), + start: 0, + end: 9, + ), + }, ); }); - testWidgetsOnAllPlatforms('inserts https scheme if it is missing', (tester) async { + testWidgetsOnAllPlatforms('recognizes a second URL when typing and converts it to a link', (tester) async { await tester // .createDocument() .withSingleEmptyParagraph() @@ -1173,25 +1170,134 @@ void main() { // Place the caret at the beginning of the empty document. await tester.placeCaretInParagraph("1", 0); - // Type a URL. It shouldn't linkify until we add a space. - await tester.typeImeText("www.google.com"); + // Type text with two URLs. + await tester.typeImeText("https://www.google.com and https://flutter.dev "); - // Type a space, to cause a linkify reaction. - await tester.typeImeText(" "); + // Ensure both URLs are linkified with the correct URLs. + final text = SuperEditorInspector.findTextInComponent("1"); + + expect(text.text, "https://www.google.com and https://flutter.dev "); + expect( + text.getAttributionSpansByFilter((a) => a is LinkAttribution), + { + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://www.google.com")), + start: 0, + end: 21, + ), + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://flutter.dev")), + start: 27, + end: 45, + ), + }, + ); + }); + + testWidgetsOnDesktop('recognizes a pasted URL with www and converts it to a link', (tester) async { + await tester // + .createDocument() + .withSingleEmptyParagraph() + .withInputSource(TextInputSource.ime) + .pump(); + + // Place the caret at the beginning of the empty document. + await tester.placeCaretInParagraph("1", 0); + + // Paste text with a URL. + tester.simulateClipboard(); + await tester.setSimulatedClipboardContent("Hello https://www.google.com world"); + // TODO: create and use something like tester.pressPasteAdaptive() + if (debugDefaultTargetPlatformOverride == TargetPlatform.macOS) { + await tester.pressCmdV(); + } else { + await tester.pressCtlV(); + } + + // Ensure the URL is linkified. + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "Hello https://www.google.com world"); + expect( + text.getAttributionSpansByFilter((a) => a is LinkAttribution), + { + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://www.google.com")), + start: 6, + end: 27, + ), + }, + ); + }); + + testWidgetsOnDesktop('recognizes a pasted URL and inserts https scheme if it is missing', (tester) async { + await tester // + .createDocument() + .withSingleEmptyParagraph() + .withInputSource(TextInputSource.ime) + .pump(); + + // Place the caret at the beginning of the empty document. + await tester.placeCaretInParagraph("1", 0); + + // Paste text with a URL. + tester.simulateClipboard(); + await tester.setSimulatedClipboardContent("Hello www.google.com world"); + // TODO: create and use something like tester.pressPasteAdaptive() + if (debugDefaultTargetPlatformOverride == TargetPlatform.macOS) { + await tester.pressCmdV(); + } else { + await tester.pressCtlV(); + } // Ensure it's linkified with a URL schema. var text = SuperEditorInspector.findTextInComponent("1"); text = SuperEditorInspector.findTextInComponent("1"); - expect(text.text, "www.google.com "); + expect(text.text, "Hello www.google.com world"); expect( - text.hasAttributionsThroughout( - attributions: { - LinkAttribution.fromUri(Uri.parse("https://www.google.com")), - }, - range: SpanRange(0, text.length - 2), - ), - isTrue, + text.getAttributionSpansByFilter((a) => a is LinkAttribution), + { + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://www.google.com")), + start: 6, + end: 19, + ), + }, + ); + }); + + testWidgetsOnDesktop('recognizes a pasted URL without https or www and converts it to a link', (tester) async { + await tester // + .createDocument() + .withSingleEmptyParagraph() + .withInputSource(TextInputSource.ime) + .pump(); + + // Place the caret at the beginning of the empty document. + await tester.placeCaretInParagraph("1", 0); + + // Paste text with a URL. + tester.simulateClipboard(); + await tester.setSimulatedClipboardContent("Hello google.com world"); + // TODO: create and use something like tester.pressPasteAdaptive() + if (debugDefaultTargetPlatformOverride == TargetPlatform.macOS) { + await tester.pressCmdV(); + } else { + await tester.pressCtlV(); + } + + // Ensure the URL is linkified. + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "Hello google.com world"); + expect( + text.getAttributionSpansByFilter((a) => a is LinkAttribution), + { + AttributionSpan( + attribution: LinkAttribution.fromUri(Uri.parse("https://google.com")), + start: 6, + end: 15, + ), + }, ); }); @@ -1211,7 +1317,7 @@ void main() { // Type some text by simulating hardware keyboard key presses. await tester.typeKeyboardText('Go to '); - // Ensure that the link is unchanged + // Ensure that the link is unchanged. expect( SuperEditorInspector.findDocument(), equalsMarkdown("Go to [www.google.com](www.google.com)"), @@ -1234,7 +1340,7 @@ void main() { // Type some text by simulating hardware keyboard key presses. await tester.typeKeyboardText('Go to '); - // Ensure that the link is unchanged + // Ensure that the link is unchanged. expect( SuperEditorInspector.findDocument(), equalsMarkdown("Go to [www.google.com](www.google.com)"), @@ -1257,7 +1363,7 @@ void main() { // Type some text by simulating hardware keyboard key presses. await tester.typeKeyboardText('Go to '); - // Ensure that the link is unchanged + // Ensure that the link is unchanged. expect( SuperEditorInspector.findDocument(), equalsMarkdown("Go to [www.google.com](www.google.com)"), @@ -1281,7 +1387,7 @@ void main() { // Type some text by simulating hardware keyboard key presses. await tester.typeKeyboardText(' to learn anything'); - // Ensure that the link is unchanged + // Ensure that the link is unchanged. expect( SuperEditorInspector.findDocument(), equalsMarkdown("[www.google.com](www.google.com) to learn anything"), @@ -1304,7 +1410,7 @@ void main() { // Type some text by simulating hardware keyboard key presses. await tester.typeKeyboardText(' to learn anything'); - // Ensure that the link is unchanged + // Ensure that the link is unchanged. expect( SuperEditorInspector.findDocument(), equalsMarkdown("[www.google.com](www.google.com) to learn anything"), @@ -1327,7 +1433,7 @@ void main() { // Type some text by simulating hardware keyboard key presses. await tester.typeKeyboardText(' to learn anything'); - // Ensure that the link is unchanged + // Ensure that the link is unchanged. expect( SuperEditorInspector.findDocument(), equalsMarkdown("[www.google.com](www.google.com) to learn anything"), @@ -1377,7 +1483,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.goog|le.com" + // Place the caret at "www.goog|le.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 8); // Add characters. @@ -1407,7 +1513,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.goog|le.com" + // Place the caret at "www.goog|le.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 8); // Add characters. @@ -1430,7 +1536,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "|www.google.com" + // Place the caret at "|www.google.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 0); // Delete downstream characters. @@ -1465,7 +1571,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "|www.google.com" + // Place the caret at "|www.google.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 0); // Delete downstream characters. @@ -1523,7 +1629,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "|www.google.com" + // Place the caret at "|www.google.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 0); // Delete downstream characters. @@ -1549,7 +1655,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google.com|" + // Place the caret at "www.google.com|". await tester.placeCaretInParagraph(doc.nodes.first.id, 10); // Delete upstream characters. @@ -1583,7 +1689,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google|.com" + // Place the caret at "www.google|.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 10); // Remove characters. @@ -1621,7 +1727,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google|.com" + // Place the caret at "www.google|.com". await tester.placeCaretInParagraph(doc.nodes.first.id, 10); // Remove a single character. @@ -1644,7 +1750,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google.com|" + // Place the caret at "www.google.com|". await tester.placeCaretInParagraph(doc.nodes.first.id, 14); // Delete upstream characters. @@ -1679,7 +1785,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google.com|" + // Place the caret at "www.google.com|". await tester.placeCaretInParagraph(doc.nodes.first.id, 14); // Delete upstream characters. @@ -1710,7 +1816,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google.com|" + // Place the caret at "www.google.com|". await tester.placeCaretInParagraph(doc.nodes.first.id, 14); // Delete an upstream characters. @@ -1815,7 +1921,7 @@ void main() { final doc = SuperEditorInspector.findDocument()!; - // Place the caret at "www.google.com|" + // Place the caret at "www.google.com|". await tester.placeCaretInParagraph(doc.nodes.first.id, 14); // Delete a character at the end of the link.