Repository navigation
[camera_windows] Fall back to temp dir for capture path. - #12755
auto-submit[bot] merged 4 commits into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Windows camera plugin to fall back to the system temp directory for saving captured pictures and videos when the standard Pictures or Videos known folders cannot be resolved. The feedback suggests updating a comment to clarify that this fallback will not be triggered if SHGetKnownFolderPath succeeds but write access is blocked, such as by Controlled Folder Access.
|
@llfbandit Could you rebase on |
mattkae
left a comment
mattkae
left a comment
There was a problem hiding this comment.
Looks good, just needs a rebase. Thanks!
ce6fa5e to
7f57de3
Compare
mattkae
left a comment
mattkae
left a comment
There was a problem hiding this comment.
@llfbandit Coming back to this - I wonder if we can write tests for this too? Not sure if there were prior ones, but it would be helpful since it's complex
7f57de3 to
3019782
Compare
|
@mattkae I had to refactor the code. I moved the known folder lookup into a virtual method to allow mocking. |
loic-sharma
left a comment
loic-sharma
left a comment
There was a problem hiding this comment.
This looks good to me, modulo minor feedback
3019782 to
f350514
Compare
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
loic-sharma
left a comment
loic-sharma
left a comment
There was a problem hiding this comment.
@llfbandit This looks good to me, but unfortunately there's a merge conflict. Could you address that? We'll get this merged in after :)
|
@loic-sharma This is unclear to me. Do you want to change to 0.4.0 and remove "NEXT" ? We're currently on version 0.3.0+1... |
|
@llfbandit Yes that's right |
Move GetCaptureDirectory into CameraPlugin and add a virtual GetKnownFolderPath so tests can mock the known folder lookup.
f350514 to
4a99ca9
Compare
Co-authored-by: Loïc Sharma <[email protected]>
loic-sharma
added
CICD
…lutter#196558) Manually rolls flutter/packages from 14f9933a8cd6 to 015df97cbc3f (5 revisions). This is the same one-line change the [flutter/packages → flutter/flutter autoroller](https://autoroll.skia.org/r/flutter-packages-flutter-autoroll) makes. Today's two automatic rolls, flutter#196527 and flutter#196547, failed on presubmit jobs that don't use the packages pin (`Windows tool_integration_tests_5` and `Linux_android_emu android_engine_opengles_tests`). The roller closed the second one after its roll window ended. It only rolls on weekdays and doesn't support manual rolls, so its next roll would be on Monday. flutter/packages@14f9933...015df97 2026-11-13 [camera_windows] Fall back to temp dir for capture path. (flutter/packages#12755) 2026-11-13 [video_player] Convert event channel to Swift and Pigeon (flutter/packages#13375) 2026-11-13 [material_ui] Document and demonstrate safe MaterialBanner dismissal across routes (flutter/packages#13267) 2026-11-12 [flutter_svg] Add a byte budget for the compiled SVG cache (flutter/packages#13596) 2026-11-12 Roll Flutter from b98afb5 to 9c427b5 (29 revisions) (flutter/packages#13636)
Fixes flutter/flutter#192302
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.No test added since there's actually no test on this path.
Footnotes
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