Skip to content

[java] Deduplicate Unicode PUA mappings in Keys; make OPTION an alias of ALT and deprecate FN#17147

Open
seethinajayadileep wants to merge 3 commits intoSeleniumHQ:trunkfrom
seethinajayadileep:fix-duplicate-keys
Open

[java] Deduplicate Unicode PUA mappings in Keys; make OPTION an alias of ALT and deprecate FN#17147
seethinajayadileep wants to merge 3 commits intoSeleniumHQ:trunkfrom
seethinajayadileep:fix-duplicate-keys

Conversation

@seethinajayadileep
Copy link
Contributor

@seethinajayadileep seethinajayadileep commented Feb 27, 2026

💥 What does this PR do?

Removes duplicate Unicode Private Use Area (PUA) mappings in org.openqa.selenium.Keys.

  • Updates OPTION to be an alias of ALT using OPTION(Keys.ALT) instead of duplicating RIGHT_ALT (\uE052).
  • Converts FN into a deprecated alias of RIGHT_CONTROL using FN(Keys.RIGHT_CONTROL) instead of directly mapping \uE051.
  • Updates Javadoc to reflect the change.

🔧 Implementation Notes

Keys.getKeyFromUnicode() returns the first matching enum value. When multiple enum constants share the same Unicode PUA value:

  • The lookup becomes ambiguous.
  • One constant becomes unreachable.
  • Behavior becomes order-dependent.

This change ensures that each Unicode PUA value maps to a single canonical key.

OPTION remains a platform-friendly alias without introducing a duplicate Unicode value.

FN is retained for backward compatibility but deprecated. It now delegates to RIGHT_CONTROL instead of defining its own duplicate Unicode value.


💡 Additional Considerations

  • No breaking API changes.
  • No duplicate Unicode values remain.
  • Backward compatibility preserved.

🔄 Types of changes

  • Bug fix (backwards compatible)

@qodo-code-review
Copy link
Contributor

PR Type

Bug fix


Description

  • Removes duplicate Unicode PUA mapping for FN key

  • Aliases OPTION to ALT instead of duplicating RIGHT_ALT

  • Updates Javadoc to clarify key specifications and aliases

  • Ensures each Unicode PUA value maps to single canonical key


File Walkthrough

Relevant files
Bug fix
Keys.java
Remove FN key and alias OPTION to ALT                                       

java/src/org/openqa/selenium/Keys.java

  • Removed FN key enum constant that duplicated RIGHT_CONTROL (\uE051)
  • Changed OPTION from duplicate RIGHT_ALT (\uE052) to alias of ALT using
    Keys.ALT
  • Updated Javadoc to clarify that OPTION is a symbolic alias and FN is
    no longer supported
  • Added explanatory comment noting FN removal rationale (not W3C spec
    compliant)
+7/-5     

@selenium-ci selenium-ci added the C-java Java Bindings label Feb 27, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Feb 27, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Feb 27, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent alias from overriding mapping

In the reverse lookup map initialization, use putIfAbsent instead of put to
prevent the OPTION alias from overwriting the ALT key's mapping.

java/src/org/openqa/selenium/Keys.java [205-207]

 for (Keys key : values()) {
-    keyMap.put(key.keyCode, key);
+    keyMap.putIfAbsent(key.keyCode, key);
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug where the new OPTION alias would overwrite the original ALT in the reverse lookup map, and proposes a correct fix using putIfAbsent.

Medium
  • Update

@seethinajayadileep seethinajayadileep changed the title Fix duplicate Unicode mappings in Keys enum (remove FN, alias OPTION to ALT) [java] Fix duplicate Unicode mappings in Keys enum (remove FN, alias OPTION to ALT) Feb 28, 2026
@VietND96 VietND96 requested a review from asolntsev February 28, 2026 12:57
Copy link
Contributor

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

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

Good for me.

@seethinajayadileep
Isn't there a risk that somebody is already using constant Keys.FN?
What would be a workaround for them?

@asolntsev asolntsev added this to the 4.42.0 milestone Feb 28, 2026
@seethinajayadileep
Copy link
Contributor Author

Good for me.

@seethinajayadileep Isn't there a risk that somebody is already using constant Keys.FN? What would be a workaround for them?

You’re right — removing Keys.FN would be a source-breaking change for anyone already referencing it.

Even though FN is not part of the W3C WebDriver specification and doesn’t have reliable driver support, existing code may still compile against it.

A safer approach would be to keep FN for backward compatibility and mark it as @deprecated, documenting that it is symbolic only and not part of the standardized key set.

That way existing users are not broken, but future usage is discouraged. I can update the PR accordingly.

@seethinajayadileep seethinajayadileep changed the title [java] Fix duplicate Unicode mappings in Keys enum (remove FN, alias OPTION to ALT) [java] Deduplicate Unicode PUA mappings in Keys; make OPTION an alias of ALT and deprecate FN Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants