Skip to content

Keep user/system data out of ids in PM #4690

@Bubballoo3

Description

@Bubballoo3

This was first encountered in #4673, but after seeing it again in the failing test in #4682, (not the cause of the failing test but how I ended up looking at it), this seems to be a pattern, and likely a dangerous one. Before #4682, select widgets were edited by introducing cards for each option, and these cards were given formulaic ids like 'launchers-{widget-name}-{value}. This posed a problem with auto accounts because we cannot guarantee account values are safe for html ids (much more stringent than html-safe).

The second instance involves the environment variable option for launchers, where as the variable name is typed, the input element for the value has its id constantly updated to match the name the user is supplying. The concept of updating an id as the user types is already a little strange, but unless I am missing some implicit advantage to this setup, it seems like the user can easily mess up this id by passing special characters or spaces. Even if this is an implicit check for environment variable name conventions, why risk having broken html instead of checking the user-supplied value against some ruleset? I haven't looked too deeply into the causes of this but it was alarming to stumble upon.

After encountering this twice, I think it would be worth reviewing the project manager code more broadly to see if there are cases where we are using ids improperly (like passing data via id) or just using user or system supplied values out of convenience to get something unique (or locate elements in tests in the first case above). In both cases we should use alternatives that give us more control over the html and error behavior. Open to discussion on this point if this pattern was intentional, but it seems to me like convenience at the cost of robustness.

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions