Fix: Link selection and editing (#2114)

* Value setting Stateful toolbar buttons derive from base class

* Removed deprecated functions

* Move clipboard actions to QuillController

* Add: Clipboard toolbar buttons

* Translation Justify

* Translation alignJustify

* Fix: Translation en-US

* Fix Link selection and editing

---------

Co-authored-by: Douglas Ward <dward@scied.com>

lib/src/document/document.dart

@@ -192,11 +192,12 @@ class Document {
       while ((res.node as Line).length == 1 && index > 0) {
         res = queryChild(--index);
       }
-      // Get inline attributes from previous line
+      // Get inline attributes from previous line (link does not cross line breaks)
       final prev = (res.node as Line).collectStyle(res.offset, 0);
       final attributes = <String, Attribute>{};
       for (final attr in prev.attributes.values) {
-        if (attr.scope == AttributeScope.inline) {
+        if (attr.scope == AttributeScope.inline &&
+            attr.key != Attribute.link.key) {
           attributes[attr.key] = attr;
         }
       }
@@ -211,13 +212,15 @@ class Document {
     //
     final style = (res.node as Line).collectStyle(res.offset - 1, 0);
     final linkAttribute = style.attributes[Attribute.link.key];
-    if ((linkAttribute != null) &&
+    if (linkAttribute != null) {
-        (linkAttribute.value !=
+      if ((res.node!.length - 1 == res.offset) ||
-            (res.node as Line)
+          (linkAttribute.value !=
-                .collectStyle(res.offset, len)
+              (res.node as Line)
-                .attributes[Attribute.link.key]
+                  .collectStyle(res.offset, len)
-                ?.value)) {
+                  .attributes[Attribute.link.key]
-      return style.removeAll({linkAttribute});
+                  ?.value)) {
+        return style.removeAll({linkAttribute});
+      }
     }
     return style;
   }

lib/src/document/nodes/line.dart

@@ -383,15 +383,34 @@ base class Line extends QuillContainer<Leaf?> {
         pos += node.length;
       }
     }
-    result = result.mergeAll(style);
+
+    /// Blank lines do not have style and must get the active style from prior line
+    if (isEmpty) {
+      var prevLine = previous;
+      while (prevLine is Block && prevLine.isNotEmpty) {
+        prevLine = prevLine.children.last;
+      }
+      if (prevLine is Line) {
+        result = result.mergeAll(prevLine.collectStyle(prevLine.length - 1, 1));
+      }
+    } else {
+      result = result.mergeAll(style);
+    }
     if (parent is Block) {
       final block = parent as Block;
       result = result.mergeAll(block.style);
     }
 
-    final remaining = len - local;
+    var remaining = len - local;
-    if (remaining > 0 && nextLine != null) {
+    var nxt = nextLine;
-      final rest = nextLine!.collectStyle(0, remaining);
+
+    /// Skip over empty lines that have no attributes
+    while (remaining > 0 && nxt != null && nxt.isEmpty) {
+      remaining--;
+      nxt = nxt.nextLine;
+    }
+    if (remaining > 0 && nxt != null) {
+      final rest = nxt.collectStyle(0, remaining);
       handle(rest);
     }
 

lib/src/editor/editor.dart

@@ -911,11 +911,22 @@ class RenderEditor extends RenderEditableContainerBox
 
     final extentNode = _container.queryChild(textSelection.end, false).node;
     RenderEditableBox? extentChild = baseChild;
-    while (extentChild != null) {
+
-      if (extentChild.container == extentNode) {
+    /// Trap shortening the text of a link which can cause selection to extend off end of line
-        break;
+    if (extentNode == null) {
+      while (true) {
+        final next = childAfter(extentChild);
+        if (next == null) {
+          break;
+        }
+      }
+    } else {
+      while (extentChild != null) {
+        if (extentChild.container == extentNode) {
+          break;
+        }
+        extentChild = childAfter(extentChild);
       }
-      extentChild = childAfter(extentChild);
     }
     assert(extentChild != null);
 

lib/src/editor/raw_editor/raw_editor_state_text_input_client_mixin.dart

@@ -79,6 +79,19 @@ mixin RawEditorStateTextInputClientMixin on EditorState
       _updateComposingRectIfNeeded();
       //update IME position for Macos
       _updateCaretRectIfNeeded();
+
+      /// Trap selection extends off end of document
+      if (_lastKnownRemoteTextEditingValue != null) {
+        if (_lastKnownRemoteTextEditingValue!.selection.end >
+            _lastKnownRemoteTextEditingValue!.text.length) {
+          _lastKnownRemoteTextEditingValue = _lastKnownRemoteTextEditingValue!
+              .copyWith(
+                  selection: _lastKnownRemoteTextEditingValue!.selection
+                      .copyWith(
+                          extentOffset:
+                              _lastKnownRemoteTextEditingValue!.text.length));
+        }
+      }
       _textInputConnection!.setEditingState(_lastKnownRemoteTextEditingValue!);
     }
     _textInputConnection!.show();

lib/src/rules/insert.dart

@@ -560,16 +560,21 @@ class PreserveInlineStylesRule extends InsertRule {
     final itr = DeltaIterator(documentDelta);
     len ??= 0;
     var prev = itr.skip(len == 0 ? index : index + 1);
-    var excludeLinkAtLineStart = false;
+    var excludeLink = false;
 
     /// Process simple insertions at start of line
     if (len == 0) {
       final currLine = itr.next();
 
-      /// Trap for previous is not text with attributes
+      /// Prevent links extending beyond the link's text label.
+      excludeLink =
+          currLine.attributes?.containsKey(Attribute.link.key) != true &&
+              prev?.attributes?.containsKey(Attribute.link.key) == true;
+
+      /// Trap for previous is not text
       if (prev?.data is! String) {
         prev = currLine;
-        excludeLinkAtLineStart = true;
+        excludeLink = true;
       } else {
         final prevData = prev!.data as String;
         if (prevData.endsWith('\n')) {
@@ -580,13 +585,17 @@ class PreserveInlineStylesRule extends InsertRule {
             if (prevData.trimRight().isEmpty) {
               final back =
                   DeltaIterator(documentDelta).skip(index - prevData.length);
-              if (back != null && back.data is String) {
+
+              /// Prevent link attribute from propagating over line break
+              if (back != null &&
+                  back.data is String &&
+                  back.attributes?.containsKey(Attribute.link.key) != true) {
                 prev = back;
               }
             }
           } else {
             prev = currLine;
-            excludeLinkAtLineStart = true;
+            excludeLink = true;
           }
         }
       }
@@ -604,7 +613,7 @@ class PreserveInlineStylesRule extends InsertRule {
       return null;
     }
 
-    if (excludeLinkAtLineStart) {
+    if (excludeLink) {
       attributes.remove(Attribute.link.key);
     }
     return Delta()

test/document/document_test.dart

@@ -4,6 +4,32 @@ import 'package:test/test.dart';
 
 void main() {
   group('collectStyle', () {
+    test('No selection', () {
+      final delta = Delta()
+        ..insert('plain\n')
+        ..insert('bold\n', <String, dynamic>{'bold': true})
+        ..insert('italic\n', <String, dynamic>{'italic': true});
+      final document = Document.fromDelta(delta);
+      //
+      expect(
+          document.getPlainText(0, document.length), 'plain\nbold\nitalic\n');
+      expect(document.length, 18);
+      //
+      for (var index = 0; index < 6; index++) {
+        expect(const Style(), document.collectStyle(index, 0));
+      }
+      //
+      for (var index = 6; index < 11; index++) {
+        expect(const Style.attr({'bold': Attribute.bold}),
+            document.collectStyle(index, 0));
+      }
+      //
+      for (var index = 11; index < document.length; index++) {
+        expect(const Style.attr({'italic': Attribute.italic}),
+            document.collectStyle(index, 0));
+      }
+    });
+
     /// Lists and alignments have the same block attribute key but can have different values.
     /// Changing the format value updates the document but must also update the toolbar button state
     /// by ensuring the collectStyles method returns the attribute selected for the newly entered line.
@@ -133,5 +159,27 @@ void main() {
       //
       expect(const Style(), document.collectStyle(3, 3));
     });
+
+    /// Links do not cross a line boundary
+    /// Enter key inserts newline as plain text without inline styles.
+    /// collectStyle needs to retrieve style of preceding line
+    test('Links and line boundaries', () {
+      final delta = Delta()
+        ..insert('A link ')
+        ..insert('home page', <String, dynamic>{'link': 'https://unknown.com'})
+        ..insert('\n\nplain\n');
+      final document = Document.fromDelta(delta);
+      //
+      const linkStyle =
+          Style.attr({'link': LinkAttribute('https://unknown.com')});
+      //
+      expect(document.collectStyle(15, 0), linkStyle, reason: 'Within Link');
+      expect(document.collectStyle(16, 0), const Style(),
+          reason: 'At end of link');
+      expect(document.collectStyle(17, 0), const Style(),
+          reason: 'start of blank line');
+      expect(document.collectStyle(18, 0), const Style(),
+          reason: 'start of blank line');
+    });
   });
 }

test/document/line_test.dart

@@ -0,0 +1,83 @@
+import 'package:flutter_quill/flutter_quill.dart';
+import 'package:flutter_quill/quill_delta.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('collectStyle', () {
+    test('Simple', () {
+      final delta = Delta()
+        ..insert('First\nSecond ')
+        ..insert('Bold', <String, dynamic>{'bold': true})
+        ..insert('\n\nplain\n');
+      final document = Document.fromDelta(delta);
+      //
+      final line = document.queryChild(6).node as Line;
+      expect(line.getPlainText(0, line.length), 'Second Bold\n');
+      expect(line.length, 12);
+      //
+      expect(line.collectStyle(0, line.length), const Style());
+      expect(
+          line.collectStyle(7, 4), const Style.attr({'bold': Attribute.bold}));
+      expect(
+          line.collectStyle(7, 5), const Style.attr({'bold': Attribute.bold}),
+          reason: 'Include trailing NL');
+      expect(
+          line.collectStyle(7, 6), const Style.attr({'bold': Attribute.bold}),
+          reason: 'Spans next NL');
+      expect(line.collectStyle(7, 7), const Style(),
+          reason: 'Spans into plain text');
+      //
+      final line2 = document.queryChild(18).node as Line;
+      expect(line2.length, 1);
+      expect(
+          line2.collectStyle(0, 1), const Style.attr({'bold': Attribute.bold}),
+          reason: 'Empty line gets style from previous line');
+    });
+
+    test('Block', () {
+      final delta = Delta()
+        ..insert('first', {'bold': true})
+        ..insert('\n', {'list': Attribute.ol})
+        ..insert('second', {'bold': true})
+        ..insert('\n', {'list': Attribute.ol})
+        ..insert('third', {'italic': true})
+        ..insert('\n', {'list': Attribute.ol})
+        ..insert('\nplain\n');
+      final document = Document.fromDelta(delta);
+      //
+      const orderedList = Attribute('list', AttributeScope.block, Attribute.ol);
+      expect(document.collectStyle(0, 4),
+          const Style.attr({'bold': Attribute.bold, 'list': orderedList}));
+      //
+      final first = document.queryChild(1).node as Line;
+      expect(first.getPlainText(0, first.length), 'first\n');
+      expect(first.length, 6);
+      expect(first.collectStyle(0, 2),
+          const Style.attr({'bold': Attribute.bold, 'list': orderedList}));
+      //
+      final second = document.queryChild(6).node as Line;
+      expect(second.getPlainText(0, second.length), 'second\n');
+      expect(second.length, 7);
+      expect(second.collectStyle(2, 4),
+          const Style.attr({'bold': Attribute.bold, 'list': orderedList}));
+      //
+      expect(first.collectStyle(3, 5),
+          const Style.attr({'bold': Attribute.bold, 'list': orderedList}),
+          reason: 'spans first and second list entry');
+      expect(second.collectStyle(3, 6), const Style.attr({'list': orderedList}),
+          reason: 'spans second and third list entry');
+      //
+      final plain = document.queryChild(20).node as Line;
+      expect(plain.getPlainText(0, plain.length), 'plain\n');
+      expect(plain.length, 6);
+      expect(plain.collectStyle(2, 4), const Style());
+      //
+      final blank = document.queryChild(19).node as Line;
+      expect(blank.getPlainText(0, blank.length), '\n');
+      expect(blank.length, 1);
+      expect(blank.getPlainText(0, 1), '\n');
+      expect(blank.collectStyle(0, 1),
+          const Style.attr({'italic': Attribute.italic, 'list': orderedList}));
+    });
+  });
+}