This repository has been archived by the owner on May 1, 2024. It is now read-only.
Make cookie handling consistent and non destructive #10571
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PureWeen
requested review from
rmarinho and
samhouts
and removed request for
StephaneDelcroix
PureWeen
added
the
DO-NOT-MERGE-!!! 🛑
PureWeen
added
blocker
|
This PR is fine to review. The API 19 LR test failed because one of the buttons is semi off the screen so the app runner couldn't click on it. Pushing up a button re-arrangement which should fix |
2 tasks
samhouts
approved these changes
May 6, 2020
| { | ||
| // cookiesToKeep.Add(jCookie.Name); |
| { | ||
| bool changeCookie = true; | ||
|
|
||
| // on iOS 10 we have to rewrite all the cookies |
There was a problem hiding this comment.
this comment might need more explanation
PureWeen
commented
May 6, 2020
rmarinho
approved these changes
May 7, 2020
samhouts
pushed a commit
that referenced
this pull request
May 7, 2020
* Remove code that adjusts the frame when hiding/showing TabBar * - enable for android * Make cookie handling consistent and non destructive (#10571) fixes #10318 * Make cookie handling consistent and non destructive * - fix ios * - ios fixes * - sync fixes * - ios fixes * - fix ui tests * - fix uwp * - move buttons around * - update comments * - ios10 adjustments * - use host for key not full url * Added repro sample (#10589) fixes #8958 * Fix TemplatedItemsList incorrect grouped collection range removal (#7891) fixes #7890 * fix * test added * uitest added * ui test fix Co-authored-by: melimion <Evlentev@molot.net> * [Tizen] Update ShellRenderer for extend (#10587) * iOS 10 fix for Top Tabs on Shell (#10500) Co-authored-by: Rui Marinho <me@ruimarinho.net> Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> Co-authored-by: melimion <33512073+melimion@users.noreply.github.com> Co-authored-by: melimion <Evlentev@molot.net> Co-authored-by: Seungkeun Lee <sngn.lee@samsung.com> Co-authored-by: Rui Marinho <me@ruimarinho.net> Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com> fixes #10134
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Determining how to do handle Cookies on each platform was different. This PR brings all the platforms in line. If the user never sets the Cookies property than nothing ever happens.
This PR also removes the API
ShouldManageCookieswhich isn't public yet. Because if the CookieContainer is null that will already act as ifShouldManageCookiesis set false. Android was the only platform that was totally destructive even when Cookies was nullThis PR keeps the Cookies property in sync with the webview cookies. After the first load of a url the Cookies container (if set) will fill with cookies. At this point if you add/remove cookies it'll try to add/remove those from the webview
I also added calls to sync cookies after the navigating call this way people can modify the cookies during navigating if they need to
Issues Resolved
Platforms Affected
Testing Procedure
PR Checklist