Add user preference for quoted content visibility in emails

[Tsopic]

Summary

• Add a new preference setting that allows users to control whether quoted content in email replies/forwards is hidden by default or automatically expanded
• Default behavior preserved: quoted content is hidden by default (collapsible "..." button)
• Users can toggle this setting in Preferences to automatically show quoted content

Changes

• Add QuotedContentConfig preference model with isHiddenByDefault flag
• Add QuotedContentExtension to load config at app startup
• Update PreferencesSettingManager with getter/setter methods
• Add quotedContent option to PreferencesOptionType enum
• Update PreferencesController to handle the new local preference
• Wire up EmailView to use the preference value for enableQuoteToggle
• Add localization strings for the new setting

Test plan

• Unit tests for QuotedContentConfig model (14 tests)
• Unit tests for PreferencesSettingManager quoted content methods (7 tests)
• Manual: Verify setting appears in Preferences screen
• Manual: Toggle setting and verify email view respects the preference
• Manual: Verify setting persists across app restarts

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added auto-sync toggle for manual WebSocket control
  • Added quoted content visibility preference in email view
  • Introduced macOS and Windows desktop platform support
  • Enhanced WebSocket authentication with flexible strategy selection

• Documentation

  • Added Docker deployment configuration guide
  • Added SERVER_URL configuration documentation
  • Added WebSocket authentication proxy documentation
  • Added architectural decision record for email batch fetching optimization

• Bug Fixes & Improvements

  • Optimized email fetching with dynamic batching and per-request limits
  • Improved WebSocket capability detection and fallback handling
  • Enhanced push notification handling for foreground/background scenarios

• CI/CD

  • Expanded build caching strategy across multiple workflows
  • Added comprehensive multi-platform release automation

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

[coderabbitai]

Walkthrough

This pull request introduces multi-platform support infrastructure, WebSocket authentication refactoring, auto-sync and quoted-content preference features, email batch-fetching optimization, and comprehensive GitHub Actions workflow improvements. Changes include: adding macOS and Windows complete build configurations; implementing a WebSocket authentication strategy pattern with multiple implementations; extending the preference system with new configurations for auto-sync and quoted-content visibility; adding batch-size limiting for JMAP email retrieval; enhancing CI/CD workflows with concurrency controls and platform-specific caching; creating documentation for Docker deployment and WebSocket authentication proxies; and introducing test coverage for the new functionality alongside platform-specific FCM configuration.

Possibly related PRs

• TF-4179 Display tag list in sidebar #4181: Modifies the same localization files (lib/main/localizations/app_localizations.dart and intl_messages.arb) for adding preference-related translation keys.
• TF-4136 Add sentry for web #4137: Updates lib/features/login/data/network/interceptors/authorization_interceptors.dart, the same file where currentToken and basicAuthCredentials getters are introduced in this PR.
• AI scribe toggle #4208: Extends the preferences management infrastructure (PreferencesSettingManager, PreferencesSetting, PreferencesOptionType) following the same pattern used here for auto-sync and quoted-content configurations.

Suggested reviewers

• tddang-linagora
• hoangdat

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'Add user preference for quoted content visibility in emails' accurately and clearly describes the main change: introducing a user preference for controlling quoted content display.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.}

[coderabbitai]

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

lib/features/mailbox/presentation/mailbox_controller.dart (1)

568-585: Avoid enqueuing refresh when currentMailboxState is null (null deref risk).

_handleWebSocketMessage dereferences currentMailboxState! (Line 592). With the new behavior, a first-load refresh can enqueue a message while currentMailboxState is still null, which will throw at runtime. Add a guard (or trigger a full refresh) before enqueuing.

🛠️ Suggested fix

   if (accountId == null || session == null) {
     _newFolderId = null;
     return;
   }

+  if (currentMailboxState == null) {
+    log('MailboxController::_refreshMailboxChanges: baseline state missing; triggering full refresh');
+    refreshAllMailbox();
+    _newFolderId = null;
+    return;
+  }
+
   // Only skip if currentMailboxState is set AND equals the new state
   // This allows refresh when currentMailboxState is null (first load)
   // or when the state has actually changed
   if (currentMailboxState != null && currentMailboxState == newState) {

.github/workflows/build.yaml (1)

8-28: Update XCODE_VERSION to a currently available version; concurrency configuration is correct.

Xcode 16.0 is no longer available on macos-latest runners. GitHub rotated out Xcode 16.0 on January 6, 2025, retaining only the three most recent major versions. The setup-xcode action at line 95 will fail when attempting to select version 16.0. Update XCODE_VERSION to 16.2 or later (check the macOS image release notes for exact available versions), then verify compatibility with your iOS deployment target. This issue also appears in release.yaml and release-all-platforms.yaml.

The concurrency grouping configuration is correct and will properly cancel duplicate runs on the same branch.

lib/features/push_notification/presentation/controller/web_socket_controller.dart (1)

156-166: Skip _connectWebSocket when state is notSupported.
Once markNotSupported() is set, lifecycle/network triggers can still reconnect and churn failures. Add a guard.

🛠️ Suggested guard

   void _connectWebSocket() {
     _connectWebSocketInteractor = getBinding<ConnectWebSocketInteractor>();
     if (_connectWebSocketInteractor == null || accountId == null || session == null) {
       logWarning('WebSocketController::_connectWebSocket: Skipping');
       return;
     }
+    if (connectionState.value == WebSocketConnectionState.notSupported) {
+      log('WebSocketController::_connectWebSocket: not supported');
+      return;
+    }
     if (_isConnecting) return;

lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart (1)

403-419: Quoted-content preference loads before preferences are injected.

Line 411 calls loadQuotedContentConfig() before _handleArguments() sets up session and runs injectPreferencesBindings(). If the manager isn’t registered yet, the preference won’t load and the default will stick.

Consider moving the load to after preferences injection (e.g., right after _handleArguments() or inside _setUpComponentsFromSession after injectPreferencesBindings()).

🛠️ One possible adjustment

   `@override`
   void onReady() {
     if (PlatformInfo.isWeb) {
       listSearchFilterScrollController = ScrollController();
       twakeAppManager.setExecutingBeforeReconnect(false);
       registerReactiveObxVariableListener();
       initialTextFormattingMenuState();
     }
-    loadQuotedContentConfig();
     if (PlatformInfo.isIOS) {
       _registerPendingCurrentEmailIdInNotification();
     }
     _handleArguments();
     _loadAppGrid();
     loadAIScribeConfig();
     loadAutoSyncConfig();
+    loadQuotedContentConfig();
     super.onReady();
   }

🤖 Fix all issues with AI agents

In `@docs/configuration/docker_deployment.md`:
- Around line 101-104: The fenced code block showing the browser console error
"Failed to load resource: net::ERR_CONNECTION_REFUSED
localhost/.well-known/openid-configuration" needs a language hint to satisfy
markdownlint MD040; update that fenced block (the triple-backtick block
containing the error message) to use a language tag such as "text" (i.e., change
``` to ```text) so the block is explicitly marked and MD040 is resolved.

In `@docs/configuration/server_url_configuration.md`:
- Around line 12-14: The fenced code blocks in server_url_configuration.md are
missing language identifiers which triggers MD040; update the triple-backtick
fences for the blocks containing
"SERVER_URL=https://your-jmap-server.example.com/" and "Failed to load resource:
localhost/.well-known/openid-configuration" to include a language tag (e.g.,
```text) so the blocks become ```text ... ```; locate the blocks by their
contents (the SERVER_URL line and the Failed to load resource line) and add the
identifier to both occurrences (also at the repeated instance around lines
42-44).

In `@docs/websocket-auth-proxy.md`:
- Around line 17-20: Update the three fenced code blocks in
docs/websocket-auth-proxy.md that currently lack a language tag (the
ASCII-diagram and the two other small blocks) by adding a language specifier
such as "text" (e.g., change ``` to ```text) so they render with proper syntax
highlighting; locate the blocks by searching for the ASCII diagram "Browser →
TLS Proxy → ws-auth-proxy → Mail Server" and the other two short fenced blocks
and prepend the language identifier to each opening fence.

In `@lib/features/base/mixin/mail_api_mixin.dart`:
- Around line 299-315: The loop in getEmailsByIdsBatched can hang or crash if
batchSize is zero/negative because maxObjectsInGet may be <= 0; before the
for-loop validate/clamp maxObjectsInGet (computed from batchSize ??
getMaxObjectsInGetMethod(session, accountId)) to a minimum of 1 (or return an
error) and use that safe value in the loop and in the logging/quotient
calculation (referencing maxObjectsInGet, batchSize, getMaxObjectsInGetMethod,
and getEmailsByIdsBatched) so start += maxObjectsInGet always makes progress and
start ~/ maxObjectsInGet is safe.

In `@lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart`:
- Around line 458-468: The tooltip strings in the WebSocketConnectionState
switch (variables tooltipMessage and cases in MailboxDashboardViewWeb) are
hard-coded English; replace them with localized lookups (e.g.,
AppLocalizations.of(context).connecting, .autoSyncNotSupported,
.disconnectedClickToRetry or similarly named keys) and add those keys to the
ARB/localization files; update usages in the cases for connecting, notSupported
and disconnected (including the disabled message that already uses
AppLocalizations) and ensure the localization import/lookup is present where
tooltipMessage is set.
- Around line 452-471: The switch on connectionState in
mailbox_dashboard_view_web.dart is missing terminators causing fall-through;
update each case in the switch (WebSocketConnectionState.connected, .connecting,
.notSupported, .disconnected) to end with an explicit terminator (e.g., add
break; after setting tooltipMessage, iconPath, iconColor) or convert each case
block to return the desired widget/value; ensure you keep the assignments to
tooltipMessage, iconPath, iconColor (and use
isEnabled/AppLocalizations/controller.imagePaths as currently referenced) before
the break/return so the compiler accepts the switch.

In
`@lib/features/push_notification/data/datasource/strategies/ticket_auth_strategy.dart`:
- Around line 21-26: In buildConnectionUri
(TicketAuthStrategy::buildConnectionUri) stop concatenating the ticket via
string interpolation which drops existing query params and can create invalid
URIs; instead take the provided baseUri and use Uri.replace (or your
WebSocketUriBuilder) to merge/append the ticket into queryParameters (e.g.,
preserve baseUri.queryParameters and add 'ticket': webSocketTicket) so the
ticket is percent-encoded and existing params remain intact before returning the
new Uri.

In `@lib/features/push_notification/presentation/services/fcm_receiver_io.dart`:
- Around line 43-60: The code currently logs raw FCM tokens in _getInitialToken,
_onHandleFcmToken, and FcmService.handleToken; remove the direct token
interpolation and instead log only non-sensitive metadata (e.g., "token
received", "token refreshed", or a masked/hashed identifier) so the full token
is never emitted; update the log calls in _getInitialToken(),
_onHandleFcmToken() (onTokenRefresh listener) and FcmService.handleToken() to
omit $token/$newToken or replace with a one-way hash or fixed message indicating
success/changed token without printing the value.

In `@lib/l10n/intl_fr.arb`:
- Around line 5501-5511: The commit contains manual additions to the French ARB
keys "autoSyncEnabled" and "autoSyncDisabled" in intl_fr.arb which violates the
localization automation; revert these manual edits and remove the two keys from
this file, then trigger or wait for the i18n extraction/translation pipeline to
regenerate locale files, or if a manual change is absolutely required, add a
brief note in the PR describing why automation was bypassed and include
instructions for translators and the i18n pipeline (reference the keys
"autoSyncEnabled" and "autoSyncDisabled" when documenting).

In `@macos/Runner/Release.entitlements`:
- Around line 5-6: The Release.entitlements currently enables app sandboxing but
omits the network client entitlement needed for outbound connections; update the
entitlements plist by adding the key com.apple.security.network.client with a
true value alongside com.apple.security.app-sandbox (ensure it’s a sibling key
in the same plist) so Twake Mail can make outbound JMAP/network requests.

In
`@test/features/push_notification/data/datasource_impl/web_socket_datasource_impl_test.dart`:
- Around line 223-307: The tests trigger real network connections because
WebSocketDatasourceImpl calls WebSocketChannel.connect directly in
getWebSocketChannel; add a constructor parameter (e.g., webSocketChannelFactory)
to WebSocketDatasourceImpl defaulting to WebSocketChannel.connect, use that
factory instead of calling WebSocketChannel.connect inline, and update tests to
inject a mock factory that returns a mocked WebSocketChannel so capability
validation can be tested without real network I/O; reference
WebSocketDatasourceImpl, getWebSocketChannel, and WebSocketChannel.connect when
making these changes.

In `@windows/.gitignore`:
- Around line 1-17: Update the windows/.gitignore to include standard
Windows/Visual Studio build and IDE artifacts missing from the current list: add
entries for .vs/, obj/, bin/, and binary/debug patterns such as *.pdb, *.dll,
*.obj, *.lib, *.ilk, *.idb, *.exp, plus Visual Studio DB files like *.VC.db and
*.VC.VC.opendb so build outputs and IDE files are ignored alongside the existing
flutter/ephemeral/ and user-specific patterns.

In `@windows/flutter/plugin_loader.cmake`:
- Line 49: The message currently uses a non-existent variable
CMAKE_CURRENT_LIST_LENGTH so it prints an empty count; replace that by computing
the actual list length (e.g., call list(LENGTH <your plugin list variable>
plugin_count)) and then use ${plugin_count} in the message(STATUS ...) call so
the line that contains message(STATUS "Loaded ${CMAKE_CURRENT_LIST_LENGTH}
plugins for Windows build") reports the real number of plugins.

In `@windows/runner/flutter_window.cpp`:
- Around line 50-67: In FlutterWindow::MessageHandler, the WM_FONTCHANGE case
calls flutter_controller_->engine()->ReloadSystemFonts() without verifying
flutter_controller_ and its engine() are non-null, risking a null dereference
during teardown; update the WM_FONTCHANGE handling to first check that
flutter_controller_ is non-null and that flutter_controller_->engine() returns a
valid pointer before calling ReloadSystemFonts(), mirroring the earlier guard
used for HandleTopLevelWindowProc (and consider referencing OnDestroy where
flutter_controller_ is reset to nullptr to understand lifecycle).

In `@windows/runner/utils.cpp`:
- Around line 10-22: The CreateAndAttachConsole function is using inverted
checks for freopen_s: it currently calls _dup2 when freopen_s returns non-zero
(failure). Change the logic so _dup2 is called only when freopen_s returns 0
(success); specifically in CreateAndAttachConsole check the return value of
freopen_s for equality to 0 before calling _dup2, and for the stderr branch
ensure you call _dup2(_fileno(stderr), 2) (not duplicating stdout) to correctly
redirect stderr. Locate these calls to freopen_s and _dup2 in
CreateAndAttachConsole and invert the conditions accordingly.

In `@windows/runner/win32_window.cpp`:
- Line 221: The call to DefWindowProc in MessageHandler is using the member
window_handle_ instead of the hwnd parameter; replace the use of window_handle_
with the hwnd argument so DefWindowProc(window_handle_, message, wparam, lparam)
becomes DefWindowProc(hwnd, message, wparam, lparam) inside MessageHandler to
avoid using a null/invalid member during destruction.
- Around line 89-107: The GetWindowClass implementation in
WindowClassRegistrar::GetWindowClass calls RegisterClass but doesn't check its
return value; update this function to verify RegisterClass returned non-zero,
and if it fails call GetLastError(), log or propagate the error (do not set
class_registered_ on failure), and return a failure indicator (e.g., nullptr or
otherwise signal to callers that kWindowClassName is not available) so
subsequent CreateWindow calls don't proceed blindly; specifically modify the
RegisterClass call site in WindowClassRegistrar::GetWindowClass (which sets
window_class fields including lpfnWndProc = Win32Window::WndProc and uses
kWindowClassName) to handle the 0 return, preserve class_registered_ semantics,
and include the error info from GetLastError() in your error handling.

In `@windows/runner/win32_window.h`:
- Around line 67-72: Update the OnCreate comment to reference the actual API
functions Create and Show (not "CreateAndShow"); modify the comment above
virtual bool OnCreate() to say it is called when Create or Show is invoked so
subclasses can perform window-related setup and return false on failure,
referencing OnCreate, Create, and Show to locate the code.
- Around line 15-26: The Point and Size structs in win32_window.h use unsigned
int which will underflow for negative window origins; change both member types
and their constructors to use signed int (matching Win32 POINT/LONG semantics)
so x, y, width, and height are signed; update the Point(unsigned int x, unsigned
int y) and Size(unsigned int width, unsigned int height) constructors to accept
int parameters and initialize the signed members accordingly, and run a build to
fix any callsites that assumed unsigned.

♻️ Duplicate comments (1)

.github/workflows/analyze-test.yaml (1)

66-67: Same cache-key reuse suggestion as in gh-pages.

See earlier note about removing github.sha from the codegen cache key to improve reuse across reruns.

🧹 Nitpick comments (13)

macos/Podfile (1)

1-47: Consider centralizing the macOS deployment target constant.
11.0 is repeated in multiple places; a single constant reduces drift risk.

♻️ Suggested refactor

+MACOS_DEPLOYMENT_TARGET = '11.0'
-
-platform :osx, '11.0'
+platform :osx, MACOS_DEPLOYMENT_TARGET
...
-      config.build_settings['MACOSX_DEPLOYMENT_TARGET'] = '11.0'
+      config.build_settings['MACOSX_DEPLOYMENT_TARGET'] = MACOS_DEPLOYMENT_TARGET

test/features/thread/presentation/controller/thread_controller_test.dart (1)

489-502: Prefer deterministic wait over a fixed delay in the new test.
Future.delayed can be flaky on CI; wait for the interactor call instead.

🛠️ Suggested change

-        // Give time for the async operations to complete
-        await Future.delayed(const Duration(milliseconds: 100));
+        // Wait for the async operations to complete
+        await untilCalled(mockRefreshChangesEmailsInMailboxInteractor.execute(
+          any,
+          any,
+          any,
+          sort: anyNamed('sort'),
+          propertiesCreated: anyNamed('propertiesCreated'),
+          propertiesUpdated: anyNamed('propertiesUpdated'),
+          emailFilter: anyNamed('emailFilter'),
+        ));

lib/features/mailbox_dashboard/presentation/extensions/quoted_content_extension.dart (1)

7-17: Narrow the try/catch to the async config call.

getBinding<T>() returns null when missing, so wrapping it in a broad try/catch can mask real failures and skips an explicit null path. Consider checking for null first, then catching only the async load.

♻️ Suggested refactor

   Future<void> loadQuotedContentConfig() async {
-    try {
-      final preferencesManager = getBinding<PreferencesSettingManager>();
-      if (preferencesManager != null) {
-        final config = await preferencesManager.getQuotedContentConfig();
-        isQuotedContentHiddenByDefault.value = config.isHiddenByDefault;
-        log('QuotedContentExtension::loadQuotedContentConfig: isHiddenByDefault = ${config.isHiddenByDefault}');
-      }
-    } catch (e) {
-      log('QuotedContentExtension::loadQuotedContentConfig: error = $e');
-      isQuotedContentHiddenByDefault.value = true;
-    }
+    final preferencesManager = getBinding<PreferencesSettingManager>();
+    if (preferencesManager == null) {
+      log('QuotedContentExtension::loadQuotedContentConfig: PreferencesSettingManager not bound');
+      return;
+    }
+    try {
+      final config = await preferencesManager.getQuotedContentConfig();
+      isQuotedContentHiddenByDefault.value = config.isHiddenByDefault;
+      log('QuotedContentExtension::loadQuotedContentConfig: isHiddenByDefault = ${config.isHiddenByDefault}');
+    } catch (e) {
+      log('QuotedContentExtension::loadQuotedContentConfig: error = $e');
+      isQuotedContentHiddenByDefault.value = true;
+    }
   }

Based on learnings, avoid try/catch around getBinding<T>() and handle the null path explicitly.

lib/features/push_notification/data/datasource/web_socket_capability_provider.dart (1)

42-61: Consider consolidating capability retrieval to avoid redundant calls.

validateCapability calls both hasCapability and getCapability, which each access session.capabilities. While harmless, you could simplify by getting the capability once and checking for null:

void validateCapability(Session session) {
  final capability = getCapability(session);
  log('WebSocketCapabilityProvider::validateCapability: capability=$capability');

  if (capability == null) {
    throw WebSocketPushNotSupportedException();
  }

  log('WebSocketCapabilityProvider::validateCapability: supportsPush=${capability.supportsPush}');

  if (capability.supportsPush == false) {
    throw WebSocketPushNotSupportedException();
  }
}

That said, the current implementation with explicit hasWebSocket logging is clear and the redundancy is negligible.

lib/features/push_notification/data/utils/web_socket_uri_builder.dart (1)

33-41: Consider case-insensitive matching for sensitive parameter keys.

The sensitive key check is case-sensitive. While unlikely in practice (since your code controls the keys), some servers or proxies might normalize query parameter casing differently.

♻️ Optional defensive improvement

 static String redactSensitiveParams(Uri uri) {
-  if (!uri.queryParameters.keys.any(_sensitiveKeys.contains)) {
+  if (!uri.queryParameters.keys.any((k) => _sensitiveKeys.contains(k.toLowerCase()))) {
     return uri.toString();
   }

   final redacted = uri.queryParameters.map((key, value) =>
-    MapEntry(key, _sensitiveKeys.contains(key) ? '[REDACTED]' : value));
+    MapEntry(key, _sensitiveKeys.contains(key.toLowerCase()) ? '[REDACTED]' : value));

   return uri.replace(queryParameters: redacted).toString();
 }

Similarly update containsSensitiveParams.

lib/features/mailbox_dashboard/presentation/extensions/auto_sync/setup_auto_sync_extension.dart (2)

8-21: Consider scoping auto-sync state to the controller instance.

static Rx state is shared across all MailboxDashBoardController instances, which can leak state across accounts/tests. Prefer storing this in the controller (or a dedicated singleton service) instead of an extension-level static.

---

23-52: Avoid wrapping getBinding<T>() in try/catch; handle null explicitly.

getBinding<T>() returns null when not registered, so the try/catch adds no safety and can hide unrelated errors. Move getBinding outside the try/catch and only wrap the async preference call. Based on learnings, ...

♻️ Suggested restructuring

 Future<void> loadAutoSyncConfig() async {
-  try {
-    final preferencesManager = getBinding<PreferencesSettingManager>();
-    if (preferencesManager != null) {
-      final config = await preferencesManager.getAutoSyncConfig();
-      _isAutoSyncEnabled.value = config.isEnabled;
-      log('SetupAutoSyncExtension::loadAutoSyncConfig: isEnabled = ${config.isEnabled}');
-      _applyAutoSyncSetting(config.isEnabled);
-    }
-  } catch (e) {
+  final preferencesManager = getBinding<PreferencesSettingManager>();
+  if (preferencesManager == null) return;
+  try {
+    final config = await preferencesManager.getAutoSyncConfig();
+    _isAutoSyncEnabled.value = config.isEnabled;
+    log('SetupAutoSyncExtension::loadAutoSyncConfig: isEnabled = ${config.isEnabled}');
+    _applyAutoSyncSetting(config.isEnabled);
+  } catch (e) {
     log('SetupAutoSyncExtension::loadAutoSyncConfig: error = $e');
     _isAutoSyncEnabled.value = true;
     _applyAutoSyncSetting(true);
   }
 }

windows/runner/generated_plugin_registrant.h (1)

7-8: Duplicate header guard with windows/flutter/generated_plugin_registrant.h.

Both windows/runner/generated_plugin_registrant.h and windows/flutter/generated_plugin_registrant.h use the same header guard GENERATED_PLUGIN_REGISTRANT_. While these files are unlikely to be included together in the same translation unit, using distinct guards (e.g., RUNNER_GENERATED_PLUGIN_REGISTRANT_) would be safer and more explicit.

Suggested fix

-#ifndef GENERATED_PLUGIN_REGISTRANT_
-#define GENERATED_PLUGIN_REGISTRANT_
+#ifndef RUNNER_GENERATED_PLUGIN_REGISTRANT_
+#define RUNNER_GENERATED_PLUGIN_REGISTRANT_

And at line 15:

-#endif  // GENERATED_PLUGIN_REGISTRANT_
+#endif  // RUNNER_GENERATED_PLUGIN_REGISTRANT_

windows/flutter/plugin_loader.cmake (1)

7-33: Apply EXCLUDED_PLUGINS to the list to avoid drift.
Right now the exclusion relies on manual edits; filtering makes future updates safer.

♻️ Suggested tweak

 list(APPEND FLUTTER_PLUGIN_LIST
   app_links
   connectivity_plus
   desktop_drop
   desktop_webview_window
   flutter_inappwebview_windows
   flutter_secure_storage_windows
   permission_handler_windows
   printing
   sentry_flutter
   share_plus
   url_launcher_windows
   window_to_front
 )
+list(REMOVE_ITEM FLUTTER_PLUGIN_LIST ${EXCLUDED_PLUGINS})

windows/runner/win32_window.cpp (1)

59-87: Singleton instance is never deleted.

The WindowClassRegistrar singleton allocated with new on line 66 is never freed. This is a minor leak at process exit. Typically acceptable for process-lifetime singletons, but worth noting.

windows/runner/main.cpp (1)

16-18: Consider checking CoInitializeEx return value.

CoInitializeEx can fail (e.g., if COM is already initialized with incompatible flags). While failures are rare, checking the result improves robustness.

Proposed fix

   // Initialize COM, so that it is available for use in the library and/or
   // plugins.
-  ::CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
+  HRESULT hr = ::CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
+  if (FAILED(hr)) {
+    return EXIT_FAILURE;
+  }

windows/runner/utils.cpp (1)

48-56: Potential unsigned underflow when WideCharToMultiByte fails.

If WideCharToMultiByte returns 0 (failure), subtracting 1 from unsigned int causes underflow to UINT_MAX. While the max_size() check catches this, checking the return value before subtraction would be clearer.

Proposed fix

-  unsigned int target_length = ::WideCharToMultiByte(
+  int raw_length = ::WideCharToMultiByte(
       CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string,
-      -1, nullptr, 0, nullptr, nullptr)
-    -1; // remove the trailing null character
+      -1, nullptr, 0, nullptr, nullptr);
+  if (raw_length <= 0) {
+    return std::string();
+  }
+  size_t target_length = static_cast<size_t>(raw_length - 1); // remove trailing null
   int input_length = (int)wcslen(utf16_string);
   std::string utf8_string;
-  if (target_length == 0 || target_length > utf8_string.max_size()) {
+  if (target_length > utf8_string.max_size()) {
     return utf8_string;
   }

windows/runner/CMakeLists.txt (1)

9-18: Custom plugin registrant appropriately excludes Firebase from Windows build.

The custom generated_plugin_registrant.cc file exists and is well-maintained with clear documentation explaining its purpose. It successfully excludes Firebase (which isn't supported on Windows) while including all other necessary plugins. The approach is intentional and properly documented in the file header.

To reduce future maintenance burden, consider documenting the process for updating this file when plugins change, or implement CI tooling to detect when the custom and auto-generated versions diverge significantly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add a language hint to the fenced block to satisfy markdownlint.

This fixes MD040 (fenced code block language).

✏️ Proposed fix

-```
+```text
 Failed to load resource: net::ERR_CONNECTION_REFUSED localhost/.well-known/openid-configuration

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	**Symptom**: Browser console shows errors like:
	```
	Failed to load resource: net::ERR_CONNECTION_REFUSED localhost/.well-known/openid-configuration
	```
	**Symptom**: Browser console shows errors like:

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

102-102: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In `@docs/configuration/docker_deployment.md` around lines 101 - 104, The fenced
code block showing the browser console error "Failed to load resource:
net::ERR_CONNECTION_REFUSED localhost/.well-known/openid-configuration" needs a
language hint to satisfy markdownlint MD040; update that fenced block (the
triple-backtick block containing the error message) to use a language tag such
as "text" (i.e., change ``` to ```text) so the block is explicitly marked and
MD040 is resolved.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add language identifiers to fenced code blocks (MD040).

🛠️ Suggested fix

-```
-SERVER_URL=https://your-jmap-server.example.com/
-```
+```text
+SERVER_URL=https://your-jmap-server.example.com/
+```

-```
-Failed to load resource: localhost/.well-known/openid-configuration
-```
+```text
+Failed to load resource: localhost/.well-known/openid-configuration
+```

Also applies to: 42-44

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

12-12: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In `@docs/configuration/server_url_configuration.md` around lines 12 - 14, The
fenced code blocks in server_url_configuration.md are missing language
identifiers which triggers MD040; update the triple-backtick fences for the
blocks containing "SERVER_URL=https://your-jmap-server.example.com/" and "Failed
to load resource: localhost/.well-known/openid-configuration" to include a
language tag (e.g., ```text) so the blocks become ```text ... ```; locate the
blocks by their contents (the SERVER_URL line and the Failed to load resource
line) and add the identifier to both occurrences (also at the repeated instance
around lines 42-44).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add language specifier to code blocks for better rendering.

Several code blocks are missing language specifiers (lines 17, 38, 44). Adding a language identifier improves syntax highlighting in documentation viewers.

📝 Suggested fix

-```
+```text
 Browser → TLS Proxy → ws-auth-proxy → Mail Server
           (HTTPS)    (token→header)   (WebSocket)

Similarly for lines 38 and 44, add `text` or an appropriate language identifier.
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

17-17: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In `@docs/websocket-auth-proxy.md` around lines 17 - 20, Update the three fenced
code blocks in docs/websocket-auth-proxy.md that currently lack a language tag
(the ASCII-diagram and the two other small blocks) by adding a language
specifier such as "text" (e.g., change ``` to ```text) so they render with
proper syntax highlighting; locate the blocks by searching for the ASCII diagram
"Browser → TLS Proxy → ws-auth-proxy → Mail Server" and the other two short
fenced blocks and prepend the language identifier to each opening fence.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Guard against zero/negative batch size to avoid an infinite loop.
If batchSize is 0/negative, start += maxObjectsInGet never advances (and start ~/ maxObjectsInGet throws). Validate or clamp before looping.

🛠️ Suggested guard

-    final maxObjectsInGet = batchSize ?? getMaxObjectsInGetMethod(session, accountId);
+    final maxObjectsInGet =
+        batchSize ?? getMaxObjectsInGetMethod(session, accountId);
+    if (maxObjectsInGet <= 0) {
+      throw ArgumentError.value(
+        maxObjectsInGet,
+        'batchSize',
+        'must be greater than 0',
+      );
+    }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	final maxObjectsInGet = batchSize ?? getMaxObjectsInGetMethod(session, accountId);
	final List<Email> allEmails = [];
	final List<EmailId> allNotFoundIds = [];
	State? latestState;
	// If maxEmailsToFetch is specified, only fetch up to that many IDs
	final idsToFetch = maxEmailsToFetch != null && maxEmailsToFetch < emailIds.length
	? emailIds.sublist(0, maxEmailsToFetch)
	: emailIds;
	for (int start = 0; start < idsToFetch.length; start += maxObjectsInGet) {
	final end = (start + maxObjectsInGet < idsToFetch.length)
	? start + maxObjectsInGet
	: idsToFetch.length;
	final batch = idsToFetch.sublist(start, end);
	log('$runtimeType::getEmailsByIdsBatched:fetching batch ${start ~/ maxObjectsInGet + 1} with ${batch.length} emails (max: $maxEmailsToFetch)');
	final maxObjectsInGet =
	batchSize ?? getMaxObjectsInGetMethod(session, accountId);
	if (maxObjectsInGet <= 0) {
	throw ArgumentError.value(
	maxObjectsInGet,
	'batchSize',
	'must be greater than 0',
	);
	}
	final List<Email> allEmails = [];
	final List<EmailId> allNotFoundIds = [];
	State? latestState;
	// If maxEmailsToFetch is specified, only fetch up to that many IDs
	final idsToFetch = maxEmailsToFetch != null && maxEmailsToFetch < emailIds.length
	? emailIds.sublist(0, maxEmailsToFetch)
	: emailIds;
	for (int start = 0; start < idsToFetch.length; start += maxObjectsInGet) {
	final end = (start + maxObjectsInGet < idsToFetch.length)
	? start + maxObjectsInGet
	: idsToFetch.length;
	final batch = idsToFetch.sublist(start, end);
	log('$runtimeType::getEmailsByIdsBatched:fetching batch ${start ~/ maxObjectsInGet + 1} with ${batch.length} emails (max: $maxEmailsToFetch)');

🤖 Prompt for AI Agents

In `@lib/features/base/mixin/mail_api_mixin.dart` around lines 299 - 315, The loop
in getEmailsByIdsBatched can hang or crash if batchSize is zero/negative because
maxObjectsInGet may be <= 0; before the for-loop validate/clamp maxObjectsInGet
(computed from batchSize ?? getMaxObjectsInGetMethod(session, accountId)) to a
minimum of 1 (or return an error) and use that safe value in the loop and in the
logging/quotient calculation (referencing maxObjectsInGet, batchSize,
getMaxObjectsInGetMethod, and getEmailsByIdsBatched) so start += maxObjectsInGet
always makes progress and start ~/ maxObjectsInGet is safe.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find and check pubspec.yaml for Dart SDK version
find . -name "pubspec.yaml" -type f | head -5

Repository: linagora/tmail-flutter

Length of output: 177

---

🏁 Script executed:

# Read the actual file at the specified lines
cat -n lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart | sed -n '440,480p'

Repository: linagora/tmail-flutter

Length of output: 2118

---

🏁 Script executed:

# Check the main pubspec.yaml for SDK version
cat pubspec.yaml | grep -A 5 "sdk:"

Repository: linagora/tmail-flutter

Length of output: 272

---

🏁 Script executed:

# Also check dart version requirements more broadly
grep -r "sdk:" . -A 2 --include="pubspec.yaml" | head -30

Repository: linagora/tmail-flutter

Length of output: 857

---

🏁 Script executed:

# Check if there are any compilation errors or build outputs related to this file
# Look for any analysis or error reports
find . -type f -name "*.dart" -path "*/mailbox_dashboard*" | head -10

Repository: linagora/tmail-flutter

Length of output: 918

---

🏁 Script executed:

# Verify Dart 3.0+ switch statement rules with web search knowledge
# Also check if there's any dart analysis or linting config that might catch this
cat -n .dart_tool/analysis_options.yaml 2>/dev/null || cat -n analysis_options.yaml 2>/dev/null || echo "No analysis_options found"

Repository: linagora/tmail-flutter

Length of output: 836

---

🏁 Script executed:

# Find the WebSocketConnectionState enum definition to verify it's exhaustive
rg -A 10 "enum WebSocketConnectionState" --type dart

Repository: linagora/tmail-flutter

Length of output: 1232

---

🏁 Script executed:

# Also check if the code actually compiles by looking for any build errors or if there are tests that would fail
fd "*.g.dart" -o -path "*/mailbox_dashboard*" | head -5

Repository: linagora/tmail-flutter

Length of output: 182

---

🏁 Script executed:

# Let me verify the Dart 3.0 switch behavior by checking documentation knowledge
# and confirm if exhaustive switches really don't need breaks in Dart 3.0+
# Let's also check if there are other similar switch statements in the codebase
# that follow the same pattern without breaks

rg "switch \(" --type dart -A 15 | head -100

Repository: linagora/tmail-flutter

Length of output: 9835

---

🏁 Script executed:

# Check if there are similar exhaustive switch patterns without breaks in the codebase
# This will help confirm if this is valid Dart 3.0+ code

rg "case.*:" --type dart -B 2 -A 2 | grep -E "(case|return|iconColor|tooltipMessage)" | head -60

Repository: linagora/tmail-flutter

Length of output: 6343

---

🏁 Script executed:

# Let me specifically check the ai_scribe_menu_action patterns that show exhaustive switches without breaks
cat -n scribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dart | sed -n '1,60p'

Repository: linagora/tmail-flutter

Length of output: 2825

---

🏁 Script executed:

# Let me check the exact pattern in the mailbox_dashboard_view_web.dart to confirm if it compiles
# by looking more carefully at the switch structure

cat -n lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart | sed -n '450,475p'

Repository: linagora/tmail-flutter

Length of output: 1437

---

🏁 Script executed:

# Search for similar switch patterns that assign variables without breaks
rg "switch.*\{" --type dart -A 20 | grep -B 5 -A 15 "iconColor\|tooltipMessage" | head -80

Repository: linagora/tmail-flutter

Length of output: 6822

---

Switch cases must have terminators to compile.

In Dart, each case in a switch statement requires a terminator: break, return, throw, or continue. Without one, fall-through occurs and the compiler rejects the code. This won't compile as written.

Suggested fix

           switch (connectionState) {
             case WebSocketConnectionState.connected:
               tooltipMessage = AppLocalizations.of(context).autoSyncEnabled;
               iconPath = controller.imagePaths.icConnectedInternet;
               iconColor = Colors.green;
+              break;
             case WebSocketConnectionState.connecting:
               tooltipMessage = 'Connecting...';
               iconPath = controller.imagePaths.icConnectedInternet;
               iconColor = Colors.orange;
+              break;
             case WebSocketConnectionState.notSupported:
               tooltipMessage = 'Auto-sync not supported by server';
               iconPath = controller.imagePaths.icDialogOfflineMode;
               iconColor = Colors.red;
+              break;
             case WebSocketConnectionState.disconnected:
               tooltipMessage = isEnabled
                   ? 'Disconnected (click to retry)'
                   : AppLocalizations.of(context).autoSyncDisabled;
               iconPath = controller.imagePaths.icDialogOfflineMode;
               iconColor = Colors.grey;
+              break;
           }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	switch (connectionState) {
	case WebSocketConnectionState.connected:
	tooltipMessage = AppLocalizations.of(context).autoSyncEnabled;
	iconPath = controller.imagePaths.icConnectedInternet;
	iconColor = Colors.green;
	case WebSocketConnectionState.connecting:
	tooltipMessage = 'Connecting...';
	iconPath = controller.imagePaths.icConnectedInternet;
	iconColor = Colors.orange;
	case WebSocketConnectionState.notSupported:
	tooltipMessage = 'Auto-sync not supported by server';
	iconPath = controller.imagePaths.icDialogOfflineMode;
	iconColor = Colors.red;
	case WebSocketConnectionState.disconnected:
	tooltipMessage = isEnabled
	? 'Disconnected (click to retry)'
	: AppLocalizations.of(context).autoSyncDisabled;
	iconPath = controller.imagePaths.icDialogOfflineMode;
	iconColor = Colors.grey;
	}
	switch (connectionState) {
	case WebSocketConnectionState.connected:
	tooltipMessage = AppLocalizations.of(context).autoSyncEnabled;
	iconPath = controller.imagePaths.icConnectedInternet;
	iconColor = Colors.green;
	break;
	case WebSocketConnectionState.connecting:
	tooltipMessage = 'Connecting...';
	iconPath = controller.imagePaths.icConnectedInternet;
	iconColor = Colors.orange;
	break;
	case WebSocketConnectionState.notSupported:
	tooltipMessage = 'Auto-sync not supported by server';
	iconPath = controller.imagePaths.icDialogOfflineMode;
	iconColor = Colors.red;
	break;
	case WebSocketConnectionState.disconnected:
	tooltipMessage = isEnabled
	? 'Disconnected (click to retry)'
	: AppLocalizations.of(context).autoSyncDisabled;
	iconPath = controller.imagePaths.icDialogOfflineMode;
	iconColor = Colors.grey;
	break;
	}

🤖 Prompt for AI Agents

In `@lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart`
around lines 452 - 471, The switch on connectionState in
mailbox_dashboard_view_web.dart is missing terminators causing fall-through;
update each case in the switch (WebSocketConnectionState.connected, .connecting,
.notSupported, .disconnected) to end with an explicit terminator (e.g., add
break; after setting tooltipMessage, iconPath, iconColor) or convert each case
block to return the desired widget/value; ensure you keep the assignments to
tooltipMessage, iconPath, iconColor (and use
isEnabled/AppLocalizations/controller.imagePaths as currently referenced) before
the break/return so the compiler accepts the switch.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Inverted freopen_s success check.

freopen_s returns 0 on success and non-zero on failure. The current logic calls _dup2 when freopen_s fails, which is the opposite of intended behavior.

Proposed fix

 void CreateAndAttachConsole() {
   if (::AllocConsole()) {
     FILE *unused;
-    if (freopen_s(&unused, "CONOUT$", "w", stdout)) {
+    if (freopen_s(&unused, "CONOUT$", "w", stdout) == 0) {
       _dup2(_fileno(stdout), 1);
     }
-    if (freopen_s(&unused, "CONOUT$", "w", stderr)) {
+    if (freopen_s(&unused, "CONOUT$", "w", stderr) == 0) {
       _dup2(_fileno(stdout), 2);
     }
     std::ios::sync_with_stdio();
     FlutterDesktopResyncOutputStreams();
   }
 }

🤖 Prompt for AI Agents

In `@windows/runner/utils.cpp` around lines 10 - 22, The CreateAndAttachConsole
function is using inverted checks for freopen_s: it currently calls _dup2 when
freopen_s returns non-zero (failure). Change the logic so _dup2 is called only
when freopen_s returns 0 (success); specifically in CreateAndAttachConsole check
the return value of freopen_s for equality to 0 before calling _dup2, and for
the stderr branch ensure you call _dup2(_fileno(stderr), 2) (not duplicating
stdout) to correctly redirect stderr. Locate these calls to freopen_s and _dup2
in CreateAndAttachConsole and invert the conditions accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing error check for RegisterClass.

RegisterClass can fail (returns 0), but the result is not checked. If registration fails, subsequent CreateWindow calls will fail silently.

Proposed fix

 const wchar_t* WindowClassRegistrar::GetWindowClass() {
   if (!class_registered_) {
     WNDCLASS window_class{};
     window_class.hCursor = LoadCursor(nullptr, IDC_ARROW);
     window_class.lpszClassName = kWindowClassName;
     window_class.style = CS_HREDRAW | CS_VREDRAW;
     window_class.cbClsExtra = 0;
     window_class.cbWndExtra = 0;
     window_class.hInstance = GetModuleHandle(nullptr);
     window_class.hIcon =
         LoadIcon(window_class.hInstance, MAKEINTRESOURCE(IDI_APP_ICON));
     window_class.hbrBackground = 0;
     window_class.lpszMenuName = nullptr;
     window_class.lpfnWndProc = Win32Window::WndProc;
-    RegisterClass(&window_class);
-    class_registered_ = true;
+    if (RegisterClass(&window_class)) {
+      class_registered_ = true;
+    }
   }
   return kWindowClassName;
 }

🤖 Prompt for AI Agents

In `@windows/runner/win32_window.cpp` around lines 89 - 107, The GetWindowClass
implementation in WindowClassRegistrar::GetWindowClass calls RegisterClass but
doesn't check its return value; update this function to verify RegisterClass
returned non-zero, and if it fails call GetLastError(), log or propagate the
error (do not set class_registered_ on failure), and return a failure indicator
(e.g., nullptr or otherwise signal to callers that kWindowClassName is not
available) so subsequent CreateWindow calls don't proceed blindly; specifically
modify the RegisterClass call site in WindowClassRegistrar::GetWindowClass
(which sets window_class fields including lpfnWndProc = Win32Window::WndProc and
uses kWindowClassName) to handle the 0 return, preserve class_registered_
semantics, and include the error info from GetLastError() in your error
handling.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use hwnd parameter instead of window_handle_ member.

DefWindowProc should use the hwnd parameter passed to MessageHandler, not window_handle_. If window_handle_ is null (e.g., during destruction), this could cause undefined behavior.

Proposed fix

-  return DefWindowProc(window_handle_, message, wparam, lparam);
+  return DefWindowProc(hwnd, message, wparam, lparam);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	return DefWindowProc(window_handle_, message, wparam, lparam);
	return DefWindowProc(hwnd, message, wparam, lparam);

🤖 Prompt for AI Agents

In `@windows/runner/win32_window.cpp` at line 221, The call to DefWindowProc in
MessageHandler is using the member window_handle_ instead of the hwnd parameter;
replace the use of window_handle_ with the hwnd argument so
DefWindowProc(window_handle_, message, wparam, lparam) becomes
DefWindowProc(hwnd, message, wparam, lparam) inside MessageHandler to avoid
using a null/invalid member during destruction.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n windows/runner/win32_window.h | head -50

Repository: linagora/tmail-flutter

Length of output: 188

---

🌐 Web query:

Windows API window coordinates LONG signed integer multi-monitor negative coordinates

💡 Result:

Short answer: Windows screen/window coordinates are signed (can be negative) on multi-monitor setups; most window APIs use signed LONG/INT RECT/POINT values, while some legacy messages pack coordinates in 16‑bit fields (LPARAM/SHORT) so you must extract them as signed. See Microsoft docs below. [1][2][3][4]

Key points:

• The virtual screen (bounding box of all monitors) can include negative coordinates when the primary monitor is not at the top‑left. Microsoft explicitly warns apps to handle negative coordinates. [1]
• GetWindowRect, MonitorFromRect, MonitorFromWindow and other Win32 APIs return coordinates in screen (virtual‑screen) coordinates using RECT/POINT types (signed values). Use those APIs for positioning. [2][4]
• Some window messages (cursor positions in LPARAM, older API values) pack coordinates into 16‑bit fields; treat them as signed (use GET_X_LPARAM/GET_Y_LPARAM rather than LOWORD/HIWORD) to correctly handle negative coordinates. [3]

Sources:
[1] Microsoft — The Virtual Screen (virtual-screen coordinates and SHORT limits).
[2] Microsoft — GetWindowRect (screen coordinates returned in RECT).
[3] Microsoft — GET_X_LPARAM / GET_Y_LPARAM (extract signed coords from LPARAM; negative coords possible).
[4] Microsoft — Positioning Objects on Multiple Display Monitors (multi‑monitor guidance).

---

🌐 Web query:

Windows API SIZE struct LONG width height dimensions unsigned or signed

💡 Result:

The SIZE structure is defined as

typedef struct tagSIZE { LONG cx; LONG cy; } SIZE;

so cx and cy are signed 32-bit LONGs (range −2,147,483,648..2,147,483,647). They are signed by definition in the Win32 API (though in many uses they represent non‑negative dimensions). [1][2]

Sources:
[1] Microsoft: SIZE structure (windef.h).
[2] Microsoft: LONG / Windows fixed-precision integer types.

---

🏁 Script executed:

cd windows && find . -type f -name "*.cc" -o -name "*.h" | head -20

Repository: linagora/tmail-flutter

Length of output: 312

---

🏁 Script executed:

cat -n windows/runner/win32_window.h | head -40

Repository: linagora/tmail-flutter

Length of output: 1654

---

🏁 Script executed:

grep -r "Point(" windows/ --include="*.cc" --include="*.h" -B2 -A2

Repository: linagora/tmail-flutter

Length of output: 319

---

🏁 Script executed:

grep -r "Size(" windows/ --include="*.cc" --include="*.h" -B1 -A1

Repository: linagora/tmail-flutter

Length of output: 272

---

🏁 Script executed:

grep -r "Create(" windows/ --include="*.cc" --include="*.h" -B1 -A1

Repository: linagora/tmail-flutter

Length of output: 667

---

🏁 Script executed:

find windows -name "*.cc" -exec grep -l "Create" {} \;

Repository: linagora/tmail-flutter

Length of output: 48

---

🏁 Script executed:

cat -n windows/runner/flutter_window.h | head -50

Repository: linagora/tmail-flutter

Length of output: 1224

---

🏁 Script executed:

find windows -type f \( -name "*.cc" -o -name "*.cpp" \) | head -20

Repository: linagora/tmail-flutter

Length of output: 273

---

🏁 Script executed:

cat -n windows/runner/main.cpp

Repository: linagora/tmail-flutter

Length of output: 1632

---

Use signed coordinates for window positions.

Win32 window origins can be negative on multi-monitor setups; unsigned fields will underflow and yield huge coordinates. Windows API defines POINT with signed LONG values and SIZE with signed LONG for width/height. Change both Point and Size structs from unsigned int to int to match Windows API conventions and support negative coordinates.

Proposed fix

  struct Point {
-   unsigned int x;
-   unsigned int y;
-   Point(unsigned int x, unsigned int y) : x(x), y(y) {}
+   int x;
+   int y;
+   Point(int x, int y) : x(x), y(y) {}
  };

  struct Size {
-   unsigned int width;
-   unsigned int height;
-   Size(unsigned int width, unsigned int height)
+   int width;
+   int height;
+   Size(int width, int height)
        : width(width), height(height) {}
  };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	struct Point {
	unsigned int x;
	unsigned int y;
	Point(unsigned int x, unsigned int y) : x(x), y(y) {}
	};
	struct Size {
	unsigned int width;
	unsigned int height;
	Size(unsigned int width, unsigned int height)
	: width(width), height(height) {}
	};
	struct Point {
	int x;
	int y;
	Point(int x, int y) : x(x), y(y) {}
	};
	struct Size {
	int width;
	int height;
	Size(int width, int height)
	: width(width), height(height) {}
	};

🤖 Prompt for AI Agents

In `@windows/runner/win32_window.h` around lines 15 - 26, The Point and Size
structs in win32_window.h use unsigned int which will underflow for negative
window origins; change both member types and their constructors to use signed
int (matching Win32 POINT/LONG semantics) so x, y, width, and height are signed;
update the Point(unsigned int x, unsigned int y) and Size(unsigned int width,
unsigned int height) constructors to accept int parameters and initialize the
signed members accordingly, and run a build to fix any callsites that assumed
unsigned.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the OnCreate comment to match actual API naming.
The comment references “CreateAndShow”, but only Create/Show exist.

🤖 Prompt for AI Agents

In `@windows/runner/win32_window.h` around lines 67 - 72, Update the OnCreate
comment to reference the actual API functions Create and Show (not
"CreateAndShow"); modify the comment above virtual bool OnCreate() to say it is
called when Create or Show is invoked so subclasses can perform window-related
setup and return false on failure, referencing OnCreate, Create, and Show to
locate the code.

[chibenwa]

Please @Tsopic manage to get the last commit and no unrelated changes onto the pull request.

Cheers !