-
Notifications
You must be signed in to change notification settings - Fork 28.7k
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
Replace deprecated openURL API call #164247
Conversation
[UIApplication.sharedApplication openURL:url]; | ||
[UIApplication.sharedApplication openURL:url | ||
options:@{} | ||
completionHandler:nil]; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
tested on my device, the new API works well and can open browser when needed. |
5b33671
to
4bffc71
Compare
Nvm, i realized that i don't have a valid reason to skip the tests. will add some tests. |
@hannah-hyj Thanks for testing it. Could you stamp it? Thanks! |
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? |
This needs a rebase for the presub to pass. |
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. |
baae1f2
to
a2cd20e
Compare
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. |
a2cd20e
to
1e2e442
Compare
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. |
Could you point me to how I can do that? Thanks!
Could you elaborate more? (I'm curious) |
You can
The G3 test failure is unrelated to this PR. From the errorlog you can see something like cc @chingjun for this same testing issue. |
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
This PR replace the deprecated
openURL:
API with equivalentopenURL: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.