Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Replace deprecated openURL API call #164247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 25, 2025

Conversation

hellohuanlin
Copy link
Contributor

@hellohuanlin hellohuanlin commented Feb 26, 2025

This PR replace the deprecated openURL: API with equivalent openURL:options:completionHandler: API.

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
#164047

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

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

@github-actions github-actions bot added platform-ios iOS applications specifically engine flutter/engine repository. See also e: labels. team-ios Owned by iOS platform team labels Feb 26, 2025
[UIApplication.sharedApplication openURL:url];
[UIApplication.sharedApplication openURL:url
options:@{}
completionHandler:nil];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @hannah-hyj i may need your help testing this out on a physical device. Do you remember what you did before to trigger this line?

Copy link
Member

Choose a reason for hiding this comment

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

yes, to trigger this line, you should set up AASA file for deep link in certain domain, and not support that deep link in your flutter app, so the link will be opened again in browser.

Copy link
Member

Choose a reason for hiding this comment

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

i will help testing if this is working

@hannah-hyj
Copy link
Member

hannah-hyj commented Mar 10, 2025

tested on my device, the new API works well and can open browser when needed.

@hellohuanlin hellohuanlin changed the title fix deprecated openURL call Replace deprecated openURL API call Mar 10, 2025
@hellohuanlin hellohuanlin force-pushed the open_url_deprecation branch from 5b33671 to 4bffc71 Compare March 10, 2025 17:56
@hellohuanlin
Copy link
Contributor Author

hellohuanlin commented Mar 10, 2025

Can I ask for a test-exempt for this?

Nvm, i realized that i don't have a valid reason to skip the tests. will add some tests.

@hellohuanlin hellohuanlin marked this pull request as ready for review March 10, 2025 18:03
@hellohuanlin
Copy link
Contributor Author

@hannah-hyj Thanks for testing it. Could you stamp it? Thanks!

@loic-sharma
Copy link
Member

loic-sharma commented Mar 12, 2025

Drive by questions: Could we have caught this sooner? My understanding is this deprecated API should've produced a warning?

Are there other deprecated APIs we're using that are causing warnings?

Do we have warnings as errors disabled on Flutter iOS? If yes, should/could we enable that?

@chinmaygarde
Copy link
Member

This needs a rebase for the presub to pass.

@hellohuanlin
Copy link
Contributor Author

Drive by questions: Could we have caught this sooner? My understanding is this deprecated API should've produced a warning?

Are there other deprecated APIs we're using that are causing warnings?

Do we have warnings as errors disabled on Flutter iOS? If yes, should/could we enable that?

It's not enabled yet. I believe we still have a few usage of deprecated API in the engine. I have seen some PRs of removing some of them over the years.

@hellohuanlin hellohuanlin force-pushed the open_url_deprecation branch from baae1f2 to a2cd20e Compare March 18, 2025 18:41
@hellohuanlin hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2025
Copy link
Contributor

auto-submit bot commented Mar 18, 2025

autosubmit label was removed for flutter/flutter/164247, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@hellohuanlin hellohuanlin force-pushed the open_url_deprecation branch from a2cd20e to 1e2e442 Compare March 21, 2025 23:55
@hannah-hyj
Copy link
Member

I think you can override the google3 testing as they are expected to fail.

My other PR had the same issue, the g3 testing is failing because during presubmit, the engine artifacts just are not uploaded onto gstatic yet, so it's fully expected that it would fail.

@hellohuanlin
Copy link
Contributor Author

I think you can override the google3 testing as they are expected to fail.

Could you point me to how I can do that? Thanks!

the engine artifacts just are not uploaded onto gstatic yet, so it's fully expected that it would fail.

Could you elaborate more? (I'm curious)

@hannah-hyj
Copy link
Member

hannah-hyj commented Mar 24, 2025

Could you point me to how I can do that? Thanks!

You can Set GitHub status in frob

Could you elaborate more?

The G3 test failure is unrelated to this PR. From the errorlog you can see something like '[https://www.gstatic.com/flutter-canvaskit/1e2e442715946dac2db479a1587e6210242f38f4/chromium/canvaskit.js] - Failed to load resource: the server responded with a status of 404 (Not Found)' which is the test trying to access the engine artifacts from the gstatic, but it failed because it doesnt exist in the presubmit phase. That's all i know 😅

cc @chingjun for this same testing issue.

@hellohuanlin hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Mar 25, 2025
Merged via the queue into flutter:master with commit 8f684b7 Mar 25, 2025
172 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 29, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 29, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
zhangyuang pushed a commit to zhangyuang/flutter-fork that referenced this pull request Jun 9, 2025
This PR replace the deprecated `openURL:` API with equivalent
`openURL:options:completionHandler:` API.

*List which issues are fixed by this PR. You must list at least one
issue. An issue is not required if the PR fixes something trivial like a
typo.*
flutter#164047

*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine flutter/engine repository. See also e: labels. platform-ios iOS applications specifically team-ios Owned by iOS platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.