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

Feature: Enhanced handling of GDPR banner acceptance in CLI #823

Merged
merged 15 commits into from
Sep 6, 2024

Conversation

amovar18
Copy link
Collaborator

@amovar18 amovar18 commented Aug 29, 2024

Description

This PR allows the user to pass a selector.json file with flag -b or --button-selectors which will be used when any custom selector is used for GDPR banner acceptance.
It also adds support for major CMP's like OneTrust, Ibuenda etc.

Relevant Technical Choices

  • Use constant array for selector CSS.
  • Use constant array for Text selector.
  • Add a custom function for using selector.json provided by the user.

Testing Instructions

  • Clone this branch.
  • In the terminal run npm i && npm run build:cli:dashboard.
  • Then run the CLI on site doctolib.fr using npm run cli https://doctolib.fr -- -b selector.json.
  • Add the following in selector.json.
{
    "cssSelectors":["didomi-notice-agree-button"],
    "textSelectors": [],
    "xPath": []
}

Additional Information:

  • If the user provided selectors aren't found in the page then the CLI falls back to using major selector, if that fails then we use the text selectors in the language.

Screenshot/Screencast

Untitled-2023-06-16-1319


Checklist

  • I have thoroughly tested this code to the best of my abilities.
  • I have reviewed the code myself before requesting a review.
  • This code is covered by unit tests to verify that it works as intended.
  • The QA of this PR is done by a member of the QA team (to be checked by QA).

Fixes #810

@amovar18 amovar18 added enhancement New feature or request CLI labels Aug 29, 2024
@amovar18 amovar18 added this to the v0.11 milestone Aug 29, 2024
@amovar18 amovar18 self-assigned this Aug 29, 2024
@fellyph
Copy link
Collaborator

fellyph commented Aug 29, 2024

  1. When the user provides a selector that doesn't exist, we should give an error message:
Screenshot 2024-08-29 at 16 02 36

@fellyph
Copy link
Collaborator

fellyph commented Aug 29, 2024

  1. When the JSON has invalid properties, the CLI should handle it and trigger a error message:
Screenshot 2024-08-29 at 16 06 11

for example:

{
  "cssselectors": ["#didomi-notice-agree-button"],
  "textSelectors": [],
  "xpath": []
}

@fellyph
Copy link
Collaborator

fellyph commented Aug 29, 2024

  1. When selecting an item with xPath an error is triggered during the audit:
Screenshot 2024-08-29 at 16 56 42
{
  "cssSelectors": [],
  "textSelectors": [],
  "xPath": ["//*[@id=\"didomi-notice-agree-button\"]"]
}

@fellyph
Copy link
Collaborator

fellyph commented Aug 29, 2024

Three items listed were fixed on the latest commit:

  • When the user provides a selector that doesn't exist, we should give an error message
  • When the JSON has invalid properties, the CLI should handle it and trigger an error message
  • When selecting an item with xPath an error is triggered during the audit

@fellyph
Copy link
Collaborator

fellyph commented Aug 29, 2024

When it runs a selector.json for a sitemap, the CLI can't find the URLs on the sitemap:

Screenshot 2024-08-29 at 17 49 34

Steps to reproduce:

  1. creates a json file
{
  "cssSelectors": ["#didomi-notice-agree-button"],
  "textSelectors": [],
  "xPath": []
}
  1. run the command: npm run cli -- -s https://www.doctolib.fr/sitemap.xml -b selector.json

@amovar18
Copy link
Collaborator Author

Screenshot 2024-08-29 at 23 42 47
@fellyph not facing this issue can you check with another sitemap please?
I debugged this got the error as 403 forbidden.

@pavanpatil1
Copy link
Collaborator

Hi @amovar18, Verified the PR, Observed few things added below:


  • If the file is not provided, it currently only shows the file name. Instead, we could display the file path.
Screenshot 2024-08-30 at 11 46 53 AM

@amovar18
Copy link
Collaborator Author

@pavanpatil1 Added a commit for the 3rd feedback, but the first 2 cannot be worked on because in those sites the button is in having an anchor tag with button styling. This breaks the accessibility rules. Our tool finds GDPR banners with acceptance buttons, not anchor tags.

@fellyph
Copy link
Collaborator

fellyph commented Aug 30, 2024

@ amovar18 anchors are common; can we consider clicking on them? I think block elements like divs and other elements are a no-go.

@fellyph
Copy link
Collaborator

fellyph commented Aug 30, 2024

I have pulled the latest changes and am still getting a Zero URL counter

Command:
npm run cli -- -s https://www.doctolib.fr/sitemap.xml -b selector.json

Selector.json:

{
  "cssSelectors": ["#didomi-notice-agree-button"],
  "textSelectors": [],
  "xPath": []
}

@fellyph
Copy link
Collaborator

fellyph commented Aug 30, 2024

The issue with zero URLs is the bot prevention system:

Screenshot 2024-08-30 at 09 08 47

It is not the scope for this task but we can open an issue to handle those cases

@amovar18
Copy link
Collaborator Author

@ amovar18 anchors are common; can we consider clicking on them? I think block elements like divs and other elements are a no-go.

They are but if the detection is off then it might redirect to another page and mess up the whole analysis.

@amovar18 amovar18 requested a review from mohdsayed September 2, 2024 08:07
@amovar18 amovar18 requested a review from mayan-000 September 2, 2024 08:08
@amovar18 amovar18 marked this pull request as ready for review September 2, 2024 08:08
@amovar18 amovar18 linked an issue Sep 2, 2024 that may be closed by this pull request
5 tasks
@milindmore22 milindmore22 mentioned this pull request Sep 2, 2024
32 tasks
@mohdsayed mohdsayed changed the base branch from develop to release/v0.11.0 September 6, 2024 09:04
@mohdsayed mohdsayed changed the title Feature: Expanded support for GDPR banner acceptance feature Feature: Expand support for GDPR banner acceptance in CLI Sep 6, 2024
Copy link
Collaborator

@mohdsayed mohdsayed left a comment

Choose a reason for hiding this comment

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

Looks good

@mohdsayed
Copy link
Collaborator

@amovar18 Please resolve conflicts and we are good to merge.

@amovar18
Copy link
Collaborator Author

amovar18 commented Sep 6, 2024

@mohdsayed resolved conflicts.

@mohdsayed mohdsayed merged commit 61b3a9a into release/v0.11.0 Sep 6, 2024
4 checks passed
@mohdsayed mohdsayed deleted the ref/gdpr-banner-acceptance branch September 6, 2024 13:38
@mohdsayed mohdsayed mentioned this pull request Sep 18, 2024
@amedina amedina changed the title Feature: Expand support for GDPR banner acceptance in CLI Feature: Enhanced handling of GDPR banner acceptance in CLI Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expanded support for GDPR banner acceptance feature
5 participants