Skip to content
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

Jump to bottom

fix: name and expirationDate should be optional when setting cookie (7-1-x) #21481

Merged
merged 1 commit into from Dec 12, 2019
Merged

fix: name and expirationDate should be optional when setting cookie (7-1-x) #21481

merged 1 commit into from Dec 12, 2019

Conversation

trop[bot]
Copy link
Contributor

@trop trop bot commented Dec 11, 2019

Backport of #21477

See that PR for details.

Notes: Fix cookies.set not working correctly when name or expirationDate is omitted.

@zcbenz @electron-bot
fix: name and expirationDate should be optional when setting cookie (#…
d95a093 
…21454)

* fix: correctly set cookie date

* fix: name is not required for setting cookie

* test: clear cookie after each cookie test

* test: should test session property

* chore: style fixes
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Dec 11, 2019
@trop trop bot requested a review from zcbenz December 11, 2019 10:34
@trop trop bot mentioned this pull request Dec 11, 2019
Merged
@trop trop bot added 7-1-x backport This is a backport PR labels Dec 11, 2019
@zcbenz zcbenz changed the title fix: name and expirationDate should be optional when setting cookie (8-x-y) fix: name and expirationDate should be optional when setting cookie (7-1-x) Dec 11, 2019
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Dec 11, 2019
@codebytere
Copy link
Member

codebytere commented Dec 11, 2019 •
edited 

not ok 763 session module ses.cookies should survive an app restart for persistent partition
  AssertionError: expected '010' to equal '110'
      at Context.<anonymous> (electron\spec-main\api-session-spec.js:249:47)
      at processTicksAndRejections (internal/process/task_queues.js:85:5)

is the failure - seems unrelated but also i've never seen this before so can anyone confirm before we merge?

@zcbenz
Copy link
Member

zcbenz commented Dec 12, 2019

is the failure - seems unrelated but also i've never seen this before so can anyone confirm before we merge?

Some PRs for master are also having the same flaky failure:
#21384

I think it is caused by crashes on exit, I'll take a look at this.

@zcbenz
Copy link
Member

zcbenz commented Dec 12, 2019

Having checked the CI history of https://github.com/electron/electron/commits/master, I'm merging this since it is suffering the same flaky failure with master.

@zcbenz zcbenz merged commit 281b074 into 7-1-x Dec 12, 2019
@release-clerk
Copy link

release-clerk bot commented Dec 12, 2019

Release Notes Persisted

Fix cookies.set not working correctly when name or expirationDate is omitted.

@zcbenz zcbenz deleted the trop/7-1-x-bp-fix-name-and-expirationdate-should-be-optional-when-setting-cookie-8-x-y--1576060484614 branch December 12, 2019 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codebytere codebytere codebytere approved these changes

@zcbenz zcbenz Awaiting requested review from zcbenz

Assignees
No one assigned
Labels
7-1-x backport This is a backport PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@codebytere @zcbenz