Automattic / vip-scanner

Deprecated: Scan all sorts of themes and files and things! Use PHPCS and the VIP coding standards instead
https://automattic.com
140 stars 51 forks source link

Toolbar issues indicator isn't accurate or consistent #144

Open designsimply opened 10 years ago

designsimply commented 10 years ago

Steps to reproduce:

  1. Install a theme, we used the Headlines theme linked at the bottom of https://sp.automattic.com/themes/thread.php?t=2989 in our example
  2. Install the VIP Scanner version 0.7
  3. Go to Tools > VIP Scanner
  4. Select each review type, click the "Scan" button, and check to see if the issues indicator in the toolbar matches the issues shown on-page

Screencast: https://videos.files.wordpress.com/qB0SWsBU/vip-scanner-0-7-toolbar-indicator-test_fmt1.ogv (Length: 2:38)

lww9fwycsf-kathrynwp x2aervsbef-kathrynwp 2pcipjmmcz-karmatosed

Found while testing VIP Scanner 0.7 during a theme review learnup.

joshbetz commented 10 years ago

The menu will only show the number of issues for the default check.

designsimply commented 10 years ago

The default check seems to be the Undefined Function Check in my setup, is that always the default?

Why does the default check show a different number of issues on different page loads? You can see the numbers change from 1 to none in the screencast and from 8 to 9 in the first two screenshots above.

Aside: I think I linked the wrong screencast filetype before, sorry about that! Try this one instead, it's a proper size and much easier to follow: https://wptestervids.files.wordpress.com/2014/08/vip-scanner-0-7-toolbar-indicator-test.mov

designsimply commented 10 years ago

I probably wasn't clear enough in my original report. Kathryn and I just double checked our setups to make sure we're seeing inconsistent numbers. First, we verified we are both using the same local setup: WP 3.9.2, VIP Scanner 0.7, and Headlines Version 1.0 By Tasko.

I see 1 or no issues in the toolbar depending on the page load, here are my screenshots:

  1. https://cloudup.com/cal1Ry9dzaO (1 Issue, Undefined Function Check)
  2. https://cloudup.com/i6fi23BZOGv (No issues, VIP Theme Review)
  3. https://cloudup.com/imXCSgBmPP8 (plugins page for reference)

Kathryn sees 8 or 9 issues:

  1. https://cloudup.com/ce2JFbsxCxZ (9 Issues, Undefined Function Check)
  2. https://cloudup.com/cfCDPWp_99Z (9 Issues, WP.com Theme Review)
  3. https://cloudup.com/cTBbwWoSo14 (8 issues, VIP Theme Review)
  4. https://cloudup.com/cOM_WIdjd8E (plugins page for reference)

In her case, the default check (which I think is the Undefined Function Check?) shows "1 Errors" in a tab on the page content area but the toolbar indicator says "9 issues" which doesn't seem to be the number of issues from the default check or any of the other scan types.

The number of issues shown in the toolbar was different in 3 different setups even though we all had the same WP, plugin, and theme versions installed on local setups. The only other differences I can think of between our setups is I'm using nginx, Kathryn's using MAMP, and Tammie was using VVV. Will that affect the scanner results?

Functionally, it seems to me like the number is wrong sometimes.

UX-wise, I would recommend removing the toolbar indicator or labeling it more clearly, i.e. "No issues (default check)" or "No issues (Undefined Function Check)." Or you could change the label to remove the count, i.e. "More Info" or "Scanner Issues."

joshbetz commented 10 years ago

Thanks, we'll take a look.