Skip to content

[dotnet][py][java][js]: add default as a value for SameSite#16129

Merged
navin772 merged 4 commits intoSeleniumHQ:trunkfrom
navin772:add-default-cookie-type
Aug 6, 2025
Merged

[dotnet][py][java][js]: add default as a value for SameSite#16129
navin772 merged 4 commits intoSeleniumHQ:trunkfrom
navin772:add-default-cookie-type

Conversation

@navin772
Copy link
Copy Markdown
Member

@navin772 navin772 commented Aug 5, 2025 •
edited by qodo-code-review bot
Loading

User description

🔗 Related Issues

Fixes #16128

💥 What does this PR do?

Adds default as a possible value for the Samesite attribute.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

  • Add Default value to SameSite enum for cookies

  • Implements cookie specification compliance across language bindings

Diagram Walkthrough

flowchart LR
  A["SameSite Enum"] --> B["Add Default Value"]
  B --> C["Cookie Spec Compliance"]
Loading

File Walkthrough
Relevant files
Enhancement
Cookie.cs
Add Default to SameSite enum                                                         

dotnet/src/webdriver/BiDi/Network/Cookie.cs

  • Add Default enum value to SameSite enumeration
 +2/-1     

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Aug 5, 2025
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Aug 5, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

16128 - Partially compliant

Compliant requirements:

• Add default as a possible value for the SameSite attribute (for dotnet binding)

Non-compliant requirements:

• Implement cookie specification compliance across language bindings (only dotnet shown)
• Support for dotnet, py, java, and js language bindings (only dotnet implementation visible)

Requires further human verification:

• Verification that similar changes are implemented in py, java, and js bindings
• Testing that the Default value works correctly with actual cookie operations

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Tests

The PR adds a new enum value but doesn't include any tests to verify the Default value works correctly in cookie operations or serialization scenarios.

 
    Default
}

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Aug 5, 2025 •
edited
Loading

PR Code Suggestions ✨

No code suggestions found for the PR.

@navin772 navin772 added C-py Python Bindings C-java Java Bindings C-nodejs JavaScript Bindings labels Aug 5, 2025
@navin772 navin772 requested a review from nvborisenko August 5, 2025 14:51
nvborisenko
nvborisenko approved these changes Aug 5, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@nvborisenko nvborisenko left a comment

Choose a reason for hiding this comment

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

.net part is correct, thanks.

@navin772
Copy link
Copy Markdown
Member Author

navin772 commented Aug 6, 2025

Failing js fedcm test is expected due to https://issues.chromium.org/issues/425801332

@navin772 navin772 merged commit 0d9ee19 into SeleniumHQ:trunk Aug 6, 2025
30 of 31 checks passed
@navin772 navin772 deleted the add-default-cookie-type branch August 6, 2025 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nvborisenko nvborisenko nvborisenko approved these changes

@Delta456 Delta456 Delta456 approved these changes

Assignees

No one assigned

Labels

C-dotnet .NET Bindings C-java Java Bindings C-nodejs JavaScript Bindings C-py Python Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: Add default as a value for SameSite cookie

4 participants

@navin772 @nvborisenko @Delta456 @selenium-ci