Alternative broken multi line replace without execCommand

[zatteo]

Following coderabbit comment here #4212 (comment), I tried to implement another solution with half success.

When selecting multiple lines and clicking on "Replace", it is adding only the last line because insertHtml calls a summernote method that throws when inserting the content.

Uncaught TypeError: Cannot read properties of null (reading 'insertBefore')
    at Object.wrap (VM172157 about:srcdoc:1:16795)
    at o.value (VM172157 about:srcdoc:1:29093)
    at o.value (VM172157 about:srcdoc:1:29260)
    at VM172157 about:srcdoc:1:29733
    at Array.map (<anonymous>)
    at o.value (VM172157 about:srcdoc:1:29708)
    at o.<anonymous> (VM172157 about:srcdoc:1:53426)
    at o.pasteHTML (VM172157 about:srcdoc:1:61341)
    at o.value (VM172157 about:srcdoc:1:22382)
    at S.fn.init.summernote (VM172157 about:srcdoc:1:24353)

That's why I wrap the HTML to insert in a <div> otherwise summernote insertion throw an error.

But then it was just inserting it after the selection, not deleting the selection. That's why I delete the selection content before inserting.

Both solution keep undo/redo history.

Video with more complex case

With execCommand

Screen.Recording.2025-12-18.at.16.48.20.mov

With deleting selection and inserting content (this PR)

See how a line break is added at the end 🤷 It happened this time but not in my other tests 🤷

Screen.Recording.2025-12-18.at.16.44.14.mov

Summary by CodeRabbit

Bug Fixes

• Enhanced text insertion functionality in the composer with improved handling of line breaks, which are now properly converted to HTML formatting
• Improved text selection management by automatically clearing active text selections before inserting new content
• Optimized asynchronous text insertion operations for more reliable, responsive, and predictable text editing experience

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This change introduces utilities for improved text insertion in a web-based composer. A new JavaScript constant in HtmlUtils deletes the current selection's contents, and a new StringConvert method converts text to HTML format by replacing newlines with <br> tags and wrapping in a <div>. The insertTextInEditor method is updated to use these utilities asynchronously, first clearing the selection via JavaScript before inserting the new content. The web editor widget is also updated to initialize the new deletion script.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Alternative broken multi line replace without execCommand' directly describes the main change: implementing an alternative approach to multi-line replace functionality that avoids using execCommand, which aligns with the PR's core objective and all code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

This PR has been deployed to https://linagora.github.io/tmail-flutter/4214.

[hoangdat]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart (1)

32-45: Consider adding error handling for async operations.

The method is now async but lacks try-catch blocks around the JavaScript evaluation and HTML insertion. Silent failures could make debugging difficult.

🔎 View suggested enhancement:

 void insertTextInEditor(String text) async {
-  final htmlContent = StringConvert.convertTextContentToHtmlContent(text);
+  try {
+    final htmlContent = StringConvert.convertTextContentToHtmlContent(text);

-  if (PlatformInfo.isWeb) {
-    await richTextWebController?.editorController.evaluateJavascriptWeb(
-      HtmlUtils.deleteSelectionContent.name,
-      hasReturnValue: false,
-    );
+    if (PlatformInfo.isWeb) {
+      await richTextWebController?.editorController.evaluateJavascriptWeb(
+        HtmlUtils.deleteSelectionContent.name,
+        hasReturnValue: false,
+      );

-    richTextWebController?.editorController.insertHtml(htmlContent);
-  } else {
-    richTextMobileTabletController?.htmlEditorApi?.insertHtml(htmlContent);
+      richTextWebController?.editorController.insertHtml(htmlContent);
+    } else {
+      richTextMobileTabletController?.htmlEditorApi?.insertHtml(htmlContent);
+    }
+  } catch (e) {
+    logError('$runtimeType::insertTextInEditor:Exception = $e');
   }
 }

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d2296c and f355901.

📒 Files selected for processing (4)

• core/lib/utils/html/html_utils.dart (1 hunks)
• core/lib/utils/string_convert.dart (1 hunks)
• lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart (1 hunks)
• lib/features/composer/presentation/widgets/web/web_editor_widget.dart (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-12-09T09:36:45.349Z

Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4194
File: lib/features/manage_account/presentation/manage_account_dashboard_controller.dart:174-176
Timestamp: 2025-12-09T09:36:45.349Z
Learning: In Dart/Flutter projects using GetX, do not wrap getBinding<T>() calls in try/catch since they return null when not found. Only wrap Get.find<T>() calls in try/catch because they throw if a dependency is unavailable. When a binding is optional, consider checking Get.isRegistered<T>() or handling the null/exception path gracefully instead of blindly catching, and document the expectation for failure modes where a dependency may not be registered.

Applied to files:

• lib/features/composer/presentation/widgets/web/web_editor_widget.dart
• lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
• core/lib/utils/string_convert.dart
• core/lib/utils/html/html_utils.dart

📚 Learning: 2025-12-09T12:47:45.861Z

Learnt from: zatteo
Repo: linagora/tmail-flutter PR: 4196
File: scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart:34-35
Timestamp: 2025-12-09T12:47:45.861Z
Learning: In the tmail-flutter repository, avoid suggesting replacing DioError with DioException unless there is a codebase-wide migration. This pattern applies to all Dart files; when reviewing, only propose a DioError-safe change (e.g., compatible error handling or conversion) unless a global migration is in place. Ensure consistency of DioError usage across modules and flag any deviations for a repo-wide decision.

Applied to files:

• lib/features/composer/presentation/widgets/web/web_editor_widget.dart
• lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart
• core/lib/utils/string_convert.dart
• core/lib/utils/html/html_utils.dart

📚 Learning: 2025-12-12T07:43:26.643Z

Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/extensions/handle_label_for_email_extension.dart:94-148
Timestamp: 2025-12-12T07:43:26.643Z
Learning: In lib/features/email/presentation/extensions/handle_label_for_email_extension.dart and similar keyword synchronization code, the addKeyword() function is the preferred method for adding keywords to emails. The resyncKeywords() pattern is being phased out and will be replaced in favor of addKeyword().

Applied to files:

• lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart

🔇 Additional comments (3)

core/lib/utils/html/html_utils.dart (1)

124-133: LGTM! JavaScript implementation is correct and defensive.

The deleteSelectionContent script properly checks for selection existence and range availability before deleting contents. The implementation follows the same IIFE pattern as other scripts in this file.

lib/features/composer/presentation/widgets/web/web_editor_widget.dart (1)

192-195: LGTM! Script registration follows the established pattern.

The deleteSelectionContent script is correctly registered in the webInitialScripts list, consistent with other utility scripts like collapseSelectionToEnd.

lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart (1)

32-45: Verify the occasional extra line break issue is acceptable.

The PR description mentions that this approach (deleting selection then inserting wrapped content) occasionally adds an extra line break at the end. Please confirm:

1. Is this behavior acceptable for the AI Scribe feature?
2. Has the root cause been identified?
3. Would it impact user experience significantly?

If this is a known limitation that's acceptable, consider documenting it with a comment explaining why this approach was chosen over execCommand and noting the trade-off.