Skip to content

feat: add placeholderValue to page source tree #1016

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ikharebashviliGD
Copy link

Summary
This PR adds support for the placeholderValue attribute to be included in the response from the /source (getPageSource) endpoint.

Motivation
Previously, placeholderValue was only available when requesting attributes for a specific element. However, this attribute is also valuable when analyzing the full accessibility tree returned by /source, particularly for identifying input fields and verifying placeholder text during automated accessibility testing.

By including placeholderValue in the full snapshot, we enable better support for end-to-end validations and tools that rely on the complete accessibility hierarchy.

Copy link

linux-foundation-easycla bot commented May 15, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@@ -216,13 +217,18 @@ + (NSDictionary *)dictionaryForElement:(id<FBXCElementSnapshot>)snapshot
},
FBExclusionAttributeFocused: ^{
return [@([wrappedSnapshot isWDFocused]) stringValue];
},
FBExclusionAttributePlaceholderValue: ^{
return FBValueOrNull(wrappedSnapshot.wdPlaceholderValue);

Choose a reason for hiding this comment

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

I'm not sure if it makes sense to have nulls everywhere as this attribute only makes sense for a limited range of elements

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if it makes sense to have nulls everywhere as this attribute only makes sense for a limited range of elements

Thanks for the feedback — you're absolutely right that placeholderValue only applies to a limited set of elements.
I've updated the implementation to include it only for relevant types (TextField, SecureTextField, SearchField) and excluded it entirely from the output for all others.
This ensures the /source response stays clean and focused, without unnecessary null values. Let me know if you'd prefer a different approach!

@mykola-mokhnach mykola-mokhnach self-requested a review May 16, 2025 10:24
@@ -201,7 +202,8 @@ + (NSDictionary *)dictionaryForElement:(id<FBXCElementSnapshot>)snapshot
info[@"label"] = FBValueOrNull(wrappedSnapshot.wdLabel);
info[@"rect"] = wrappedSnapshot.wdRect;

NSDictionary<NSString *, NSString * (^)(void)> *attributeBlocks = @{
// Define the base attribute blocks that apply to all elements.
NSDictionary<NSString *, NSString * (^)(void)> *baseAttributeBlocks = @{

Choose a reason for hiding this comment

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

I think it makes sense to extract this logic into a separate method for better readability.

Also this dictionary should be defined as mutable to reduce the amount of boilerplate code


// Add placeholderValue only for elements where it is meaningful (e.g., text input fields).
switch (snapshot.elementType) {
case XCUIElementTypeTextField:

Choose a reason for hiding this comment

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

consider reusing this helper for detecting whether a particular element is a text field:

Copy link
Author

Choose a reason for hiding this comment

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

@mykola-mokhnach I initially tried to use fb_supportsInnerText as suggested, but it's only available on XCUIElement, while here we work exclusively with FBXCElementSnapshotWrapper.
Rather than casting or duplicating the logic inline, I added a minimal helper fb_supportsPlaceholder to the existing FBXCElementSnapshotWrapper+Helpers category.
This keeps everything localized, avoids unnecessary switching, and makes it easier to extend later.

Copy link
Member

Choose a reason for hiding this comment

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

Would exposing fb_supportsInnerText from XCUIElement+FBWebDriverAttributes.h help to refer to it?

Choose a reason for hiding this comment

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

I've extracted this helper in the other PR. Please rebase with master to be able to use it

Moved the logic for constructing attribute blocks into a separate method
for clarity and future extensibility. Also replaced direct type checks
with a reusable fb_supportsPlaceholder helper on FBXCElementSnapshotWrapper.
@@ -248,6 +237,41 @@ + (NSDictionary *)dictionaryForElement:(id<FBXCElementSnapshot>)snapshot
return info;
}

// Private helper that builds all attribute blocks for a given snapshot.
// Includes both base attributes and any element-specific ones (e.g. placeholder for text inputs, etc.).
+ (NSMutableDictionary<NSString *, NSString *(^)(void)> *)attributeBlockMapForSnapshot:(id<FBXCElementSnapshot>)snapshot

Choose a reason for hiding this comment

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

The returned dictionary must be immutable

Choose a reason for hiding this comment

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

method names must always start with fb_ in extension classes

@@ -248,6 +237,41 @@ + (NSDictionary *)dictionaryForElement:(id<FBXCElementSnapshot>)snapshot
return info;
}

// Private helper that builds all attribute blocks for a given snapshot.

Choose a reason for hiding this comment

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

consider moving the helper closer to the tail of the class definition if it is private

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants