equalizedigital / accessibility-checker

GNU General Public License v2.0
14 stars 8 forks source link

"Null" issues reported for checks that are being disabled by a filter #524

Closed christophermhinds closed 6 months ago

christophermhinds commented 6 months ago

Please give us a description of what happened.

When a user has already run a scan and then adds a filter that disables a check, positives on disabled checks can still show up in our reports as a "null" value.

Please describe what you expected to happen and why.

The expected result of disabling a check is that the disabled check would be filtered out everywhere, including in scan results.

How can we reproduce this behavior?

  1. Run a scan on a page where a specific check will trigger a positive result.
  2. Disable the triggered check by adding a filter to the theme or functions file.
  3. Go back to the report on that page and you should see a "null" value displaying in front-end and potentially other areas of our reports.

Technical info

N/A

This is not a result of a plugin conflict.

N/A

pattonwebz commented 6 months ago

I opened a PR to fix the issue for the frontend highlight widget. There is still a situation where the issues counts in the editor do not respect the filtered rule issue totals (in situations where the last scan happened before the filtering of rules was done).

pattonwebz commented 6 months ago

I did some benchmarking on the impact of editing queries from the summary widget to avoid showing rules that have been filtered out since the last post scan and the impact is about 4% increase in query time. I don't think we should do anything in this pass to solve that because the impact is only on people who have filtered out a rule between last scan and not yet run a rescan.

I benchmarked it 2 times. Once with 100k queries and a 2nd time with 400k. So over a half million queries, the average is about 4% slower queries when they include NOT IN() checks for rules that are filtered out.

Screenshot from 2024-03-11 14-42-40

SteveJonesDev commented 6 months ago

I figured there would be a minor performance impact but thanks for being thorough and providing actual benchmarks!

@christophermhinds, This has been fixed with version 1.9.2.