Skip to content

Use ASSIGNCOPY for NSAccessibilityElement string setters#398

Closed
Copilot wants to merge 1 commit into
NSAccessibilityCustomRotor_branchfrom
copilot/sub-pr-362
Closed

Use ASSIGNCOPY for NSAccessibilityElement string setters#398
Copilot wants to merge 1 commit into
NSAccessibilityCustomRotor_branchfrom
copilot/sub-pr-362

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 17, 2026

Addresses feedback from PR #362 to use ASSIGNCOPY instead of ASSIGN for string attribute setters in NSAccessibilityElement.m to prevent issues with mutable strings being modified after assignment.

Changes

  • String setters: Updated setAccessibilityLabel:, setAccessibilityIdentifier:, setAccessibilityRole:, and setAccessibilitySubrole: to use ASSIGNCOPY
  • Initializer: Updated initWithRole:frame:label:parent: to use ASSIGNCOPY for role and label parameters
// Before
- (void) setAccessibilityLabel: (NSString *)label
{
  ASSIGN(_accessibilityLabel, label);  // retains
}

// After
- (void) setAccessibilityLabel: (NSString *)label
{
  ASSIGNCOPY(_accessibilityLabel, label);  // copies
}

This prevents potential bugs where a caller passes an NSMutableString, then mutates it after assignment.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] WIP addressing feedback on NSAccessibility implementations from PR #362 Use ASSIGNCOPY for NSAccessibilityElement string setters Feb 17, 2026
Copilot AI requested a review from gcasa February 17, 2026 04:59
@rmottola
Copy link
Copy Markdown
Member

Does this make sens at all at a first glance? parameters are NSStrings and not mutable, aren't they? is there a deeper issue?

@fredkiefer
Copy link
Copy Markdown
Member

As the AI already explained, the parameter could as well be an NSMutableString, which is a sub class of NSString (Actually it is more complicated, as NSString is a class cluster, but we will ignore that for now).
The good thing here is, that for NSString copying is a very cheap operation, it will just return the object itself.

@gcasa gcasa marked this pull request as ready for review February 20, 2026 04:04
@gcasa
Copy link
Copy Markdown
Member

gcasa commented Feb 20, 2026

This PR is outdated since I moved these into another class.

@gcasa gcasa closed this Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants