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

Excluding all logged in users from Analytics results in invalid JSON in Web Stories preview debug mode #3572

Closed
LuckynaSan opened this issue Jun 14, 2021 · 6 comments
Labels
P2 Low priority Rollover Issues which role over to the next sprint Type: Bug Something isn't working
Milestone

Comments

@LuckynaSan
Copy link
Collaborator

LuckynaSan commented Jun 14, 2021

Bug Description

1.) When you enable the option to exclude all logged in users from Analytics in Site Kit by Google, Story passes AMP validator, but Story's debug mode in the story preview is outputting the below warning
Screen Shot 2021-06-11 at 12 06 36 PM
Screen Shot 2021-06-11 at 12 08 40 PM

Steps to reproduce

  1. Go to Site Kit settings
  2. Click on edit Analytics > Exclude from Analytics all logged-in users
  3. Go to Web Stories
  4. Open in editor > preview a story
  5. See debug tab and notice invalid JSON error

Screenshots

Additional Context

Related Github issue GoogleForCreators/web-stories-wp#7939

  • PHP Version:
  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari] chrome
  • Plugin Version [e.g. 22] 1.34.0
  • Device: [e.g. iPhone6] macOS

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The GA opt-out element output for AMP should not raise any validation errors or warnings for HTML or AMP markup validation

Implementation Brief

Test Coverage

  • No changes

Visual Regression Changes

  • N/A

QA Brief

  • Set up and configure Analytics to exclude tracking for logged in users (default)
  • Enable Web Stories and create a new story
  • Leave blank and click Preview
  • On the preview screen, click the Debug tab
  • Review the results, ensure the __gaOptOutExtension element is not reported (there may be other validation errors/warnings not from Site Kit)

Changelog entry

  • Fix AMP validation error caused by the GA opt-out snippet conditionally placed by Site Kit.
@LuckynaSan LuckynaSan added the Type: Bug Something isn't working label Jun 14, 2021
@aaemnnosttv aaemnnosttv self-assigned this Jun 15, 2021
@aaemnnosttv aaemnnosttv added the P2 Low priority label Jun 17, 2021
@felixarntz
Copy link
Member

IB / CR ✅

@felixarntz felixarntz added this to the Sprint 51 milestone Jun 17, 2021
@felixarntz
Copy link
Member

@eclarke1 @fhollis Adding this to Sprint 51 since it was a quick fix.

@felixarntz felixarntz removed their assignment Jun 17, 2021
@fhollis fhollis added the Rollover Issues which role over to the next sprint label Jun 21, 2021
@fhollis fhollis modified the milestones: Sprint 51, Sprint 52 Jun 21, 2021
@wpdarren
Copy link
Collaborator

@aaemnnosttv I am seeing the same error and tested this on two different sites. Would you be able to check that it has successfully merged, and if you still see the errors or not?

image

@aaemnnosttv
Copy link
Collaborator

@wpdarren the problem is that Site Kit isn't setting the proper AMP mode when Web Stories is active. This should be fixed in #2998 but there is a problem with the implementation there which I will follow up on now.

@aaemnnosttv
Copy link
Collaborator

@wpdarren actually, sorry, it should be good – I just tested this again and while there are errors that show Site Kit, it's mostly because the output is minified and line 6 starts with our opt-out comment there which is normal. The main error we don't want to see is the first one in your screenshot there about invalid JSON.

Here's what I see
image

@wpdarren wpdarren self-assigned this Jun 23, 2021
@wpdarren
Copy link
Collaborator

QA Update: Pass ✅

  • The invalid JSON is not reported as per the QAB.

image

@wpdarren wpdarren removed their assignment Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Rollover Issues which role over to the next sprint Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants