Skip to content

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

Merged
mohdsayed merged 15 commits into
release/v0.11.0from
ref/gdpr-banner-acceptance
Sep 6, 2024
Merged

Feature: Enhanced handling of GDPR banner acceptance in CLI#823
mohdsayed merged 15 commits into
release/v0.11.0from
ref/gdpr-banner-acceptance

Conversation

@amovar18

@amovar18 amovar18 commented Aug 29, 2024

Copy link
Copy Markdown
Collaborator

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

fellyph commented Aug 29, 2024

Copy link
Copy Markdown
Contributor
  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

fellyph commented Aug 29, 2024

Copy link
Copy Markdown
Contributor
  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

fellyph commented Aug 29, 2024

Copy link
Copy Markdown
Contributor
  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

fellyph commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

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

fellyph commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

fellyph commented Aug 30, 2024

Copy link
Copy Markdown
Contributor

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

@fellyph

fellyph commented Aug 30, 2024

Copy link
Copy Markdown
Contributor

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

fellyph commented Aug 30, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Comment thread packages/analysis-utils/src/browserManagement/index.ts Outdated
Comment thread packages/analysis-utils/src/browserManagement/index.ts Outdated
Comment thread packages/cli/src/index.ts
@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

@mohdsayed mohdsayed left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator

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

@amovar18

amovar18 commented Sep 6, 2024

Copy link
Copy Markdown
Collaborator Author

@mohdsayed resolved conflicts.

@mohdsayed mohdsayed merged commit 61b3a9a into release/v0.11.0 Sep 6, 2024
@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