Skip to content

Conversation

vashworth
Copy link
Contributor

@vashworth vashworth commented May 27, 2025

file_selector_macos is trying to set nameFieldStringValue for an NSOpenPanel. However, nameFieldStringValue does not support NSOpenPanel, it only supports NSSavePanel.

This PR does not set nameFieldStringValue if using NSOpenPanel. It also updates the tests to verify it works for NSSavePanel and not NSOpenPanel.

Screenshot 2025-05-27 at 11 10 40 AM

Fixes flutter/flutter#169510.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

@stuartmorgan-g
Copy link
Collaborator

Our tests for file_selector_macos are trying to set nameFieldStringValue for an NSOpenPanel. However, nameFieldStringValue does not support NSOpenPanel, it only supports NSSavePanel.

Which is an interesting distinction for them to draw, given that they decided (oddly, IMO) to make NSOpenPanel inherit from NSSavePanel...

Screenshot 2025-05-27 at 11 10 40 AM

It's unfortunate that this was only documented in the header, and not the primary API docs 😐

Given the undocumented-anywhere-but-observed behavior that it logs a message implying that we are doing something wrong rather than silently ignoring it, we should probably change the implementation to not call the setter in this case. This is the kind of logging that leads to the situation where anyone reporting any issue related to the plugin sees what sounds like an error being logged, thinks it's related to the problem they are having, and then everyone piles every unrelated report into a single issue because they find that log string when searching the issue tracker.

@vashworth vashworth changed the title [file_selector_macos] Update test to validate nameFieldStringValue for NSSavePanel instead of NSOpenPanel [file_selector_macos] Do not set nameFieldStringValue for NSOpenPanel May 27, 2025
@vashworth
Copy link
Contributor Author

we should probably change the implementation to not call the setter in this case

Good idea. Updated the PR

@vashworth vashworth marked this pull request as ready for review May 27, 2025 16:51
@vashworth vashworth requested a review from cbracken as a code owner May 27, 2025 16:51
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM stamp from a Japanese personal seal

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label May 27, 2025
@auto-submit auto-submit bot merged commit 5743798 into flutter:main May 27, 2025
81 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request May 28, 2025
flutter/packages@6eebe72...5743798

2025-05-27 [email protected]
[file_selector_macos] Do not set nameFieldStringValue for NSOpenPanel
(flutter/packages#9324)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
…flutter#9324)

file_selector_macos is trying to set [`nameFieldStringValue`](https://developer.apple.com/documentation/appkit/nssavepanel/namefieldstringvalue) for an `NSOpenPanel`. However, `nameFieldStringValue` does not support `NSOpenPanel`, it only supports `NSSavePanel`.

This PR does not set `nameFieldStringValue` if using `NSOpenPanel`. It also updates the tests to verify it works for `NSSavePanel` and not `NSOpenPanel`.

![Screenshot 2025-05-27 at 11 10 40 AM](https://github.com/user-attachments/assets/6af2016b-da93-482d-a5a2-5aeeac1abb47)

Fixes flutter/flutter#169510.

## Pre-Review Checklist

[^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Ortes pushed a commit to Ortes/packages that referenced this pull request Jun 25, 2025
…flutter#9324)

file_selector_macos is trying to set [`nameFieldStringValue`](https://developer.apple.com/documentation/appkit/nssavepanel/namefieldstringvalue) for an `NSOpenPanel`. However, `nameFieldStringValue` does not support `NSOpenPanel`, it only supports `NSSavePanel`.

This PR does not set `nameFieldStringValue` if using `NSOpenPanel`. It also updates the tests to verify it works for `NSSavePanel` and not `NSOpenPanel`.

![Screenshot 2025-05-27 at 11 10 40 AM](https://github.com/user-attachments/assets/6af2016b-da93-482d-a5a2-5aeeac1abb47)

Fixes flutter/flutter#169510.

## Pre-Review Checklist

[^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: file_selector platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mac-15] file_selector_macos test ExampleTests.testOpenWithArguments() fails on macOS 15.5
3 participants