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

Ensure AMP is available when moderate/loose sandboxing is enabled and there are stored validation errors #7997

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

westonruter
Copy link
Member

On my site I have the Loose sandboxing mode enabled:

image

I tried enabling Transitional mode, and I was confused why accessing a page was redirecting to the non-AMP version when I was not logged-in, but I was able to access the page while logged in. The URL in question had been validated and there were validation errors captured for it:

image

It appears the problem is that the sandboxing level wasn't taken into account in the amp_is_available() function. When there are validation errors associated with a URL, normally it will redirect to the non-AMP version. But when Sandboxing is enabled and either the Loose or Moderate levels are used, this should not be happening.

This also fixes test_decorate_shortcode_and_filter_source to make it less brittle.

@westonruter westonruter added this to the v2.5.6 milestone Mar 5, 2025
@westonruter westonruter requested a review from milindmore22 March 5, 2025 20:01
@westonruter westonruter force-pushed the fix/amp-unavaiable-when-validation-errors-present-with-sandboxing branch from 469634e to 70d0086 Compare March 5, 2025 21:12
@@ -6,7 +6,7 @@ runs:
using: 'composite'
steps:
- name: Cache assets directory
uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 #v4.0.2
uses: actions/cache@v4
Copy link
Member Author

Choose a reason for hiding this comment

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

I made this change because GHA was failing with an error:

Error: This request has been automatically failed because it uses a deprecated version of actions/cache: 0c45773b623bea8c8e75f6c82b208c3cf94ea4f9. Please update your workflow to use v3/v4 of actions/cache to avoid interruptions. Learn more: https://github.blog/changelog/2024-12-05-notice-of-upcoming-releases-and-breaking-changes-for-github-actions/#actions-cache-v1-v2-and-actions-toolkit-cache-package-closing-down

I'm confused why this is happening though because actions/cache@0c45773 is v4.0.2 so it is already v4.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm also confused why Dependabot didn't update this automatically. It updated some of the workflows but not all of them: #7991

Copy link
Collaborator

@thelovekesh thelovekesh Mar 5, 2025

Choose a reason for hiding this comment

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

Actually we need to define the paths for @dependabot to look for workflow files in .github/actions/*

  - package-ecosystem: github-actions
    directory: '.github/actions/setup-bun'
    schedule:
      interval: weekly
      day: saturday
    groups:
      actions:
        patterns:
          - 'actions/*'
    labels:
      - 'github-actions'
      - 'dependencies'

Copy link
Member Author

Choose a reason for hiding this comment

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

Why wouldn't directory: "/" be handling this?

- package-ecosystem: github-actions
directory: "/"
schedule:
interval: monthly
time: "17:00"
timezone: America/Los_Angeles
open-pull-requests-limit: 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants