Skip to content
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

new_audit: paste-preventing-inputs #14313

Merged
merged 7 commits into from
Feb 1, 2023
Merged

new_audit: paste-preventing-inputs #14313

merged 7 commits into from
Feb 1, 2023

Conversation

connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Aug 23, 2022

Removes password-inputs-can-be-pasted-into and adds a more generic paste-preventing-inputs audit.

@connorjclark connorjclark requested a review from a team as a code owner August 23, 2022 18:40
@connorjclark connorjclark requested review from brendankenny and removed request for a team August 23, 2022 18:40
"description": "Preventing input pasting undermines good security policy (in the case of passwords) and is a UX anti-pattern (in the general case). [Learn more about user-friendly input fields](https://web.dev/password-inputs-can-be-pasted-into/).",
"score": null,
"scoreDisplayMode": "error",
"errorMessage": "Required Inputs gatherer did not run."
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@adamraine how should I update these FR sample artifacts? We may need a script to do this (like we have for legacy runner)

Copy link
Member

@adamraine adamraine Aug 23, 2022

Choose a reason for hiding this comment

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

yarn update:flow-sample-json --rebaseline-artifacts Inputs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm

image

Copy link
Member

Choose a reason for hiding this comment

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

Yeah maybe the script needs updating/hardening for this specific use case, but the structure is already there.

@@ -6,140 +6,160 @@
"entryType": "measure",
"startTime": 0,
"duration": 1,
"detail": null
"detail": null,
"gather": true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@adamraine are these expected changes? From running update on just Inputs.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm looks like a bug in update-flow-fixtures.js. Timing should only update if you specify it.

I'll look into it but we can probably still include it in this PR since that's the artifact we would be storing.

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, I think this is expected. loadArtifacts adds it when we pull the artifacts from disk:

// Any Timing entries in saved artifacts will have a different timeOrigin than the auditing phase
// The `gather` prop is read later in generate-timing-trace and they're added to a separate track of trace events
artifacts.Timing.forEach(entry => (entry.gather = true));

I think it makes sense to keep.

clients/test/lightrider/lightrider-entry-test.js Outdated Show resolved Hide resolved
core/audits/dobetterweb/inputs-can-be-pasted-into.js Outdated Show resolved Hide resolved
@connorjclark
Copy link
Collaborator Author

Asked ChatGPT what it thinks we should name the audit...

Lighthouse is a website audit tool comprised of many audits. Audits have an id, which ideally follow these rules:

- The audit ID should be kebal-case.
- The audit ID should be based on the noun of the subject matter that it surfaces to the user
- No verbs.
- No `no-` prefixes.
- Use the noun of the items it surfaces or concept it centers around.
- Adjective modifiers are acceptable and encouraged if the noun would be too broad without specificty.
- If an adjective modifier will result in describing either the passing or failing state, prefer the failing state.

A new audit is being developed that fails if there are any input elements on the page that prevent the user from pasting the contents
of their clipboard. This behavior results in poor user experiences, because it can be frustrating to not be able to quickly insert text. Most commonly, it makes using password managers cumbersome. But it can make any type of input harder to use, such as pasting an address.

Please give some suggestions for a good id for this new audit.

Response:

1. clipboard-pasting
2. pasteability
3. clipboard-paste-blocking
4. paste-prevention
5. clipboard-usage
6. copy-paste-accessibility

@patrickhulce
Copy link
Collaborator

ooooo, pasteability, look at chatgpt go :D

@connorjclark
Copy link
Collaborator Author

sorry chatgpt but we went with paste-preventing-inputs. you didn't account for the second item in our list of rules, shame

@connorjclark connorjclark changed the title new_audit: inputs-can-be-pasted-into new_audit: paste-preventing-inputs Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants