diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 42dcc426d0f..a8d4d9a2967 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -5,6 +5,6 @@ Fixes # ## Checklist -- [ ] My pull request is addressing an [open issue](https://github.com/ampproject/amp-wp/contributing/project-management.md#life-of-an-issue) (please create one otherwise). -- [ ] My code is tested and passes existing [tests](https://github.com/ampproject/amp-wp/contributing/engineering.md#tests). -- [ ] My code follows the [Engineering Guidelines](https://github.com/ampproject/amp-wp/contributing/engineering.md) (updates are often made to the guidelines, check it out periodically). +- [ ] My pull request is addressing an open issue (please create one otherwise). +- [ ] My code is tested and passes existing [tests](https://github.com/ampproject/amp-wp/wiki/Engineering-Guidelines#tests). +- [ ] My code follows the [Engineering Guidelines](https://github.com/ampproject/amp-wp/wiki/Engineering-Guidelines) (updates are often made to the guidelines, check it out periodically). diff --git a/assets/src/block-editor/components/amp-preview.js b/assets/src/block-editor/components/amp-preview.js index dfbace46e04..c09ed032019 100644 --- a/assets/src/block-editor/components/amp-preview.js +++ b/assets/src/block-editor/components/amp-preview.js @@ -8,7 +8,7 @@ import PropTypes from 'prop-types'; * WordPress dependencies */ import { Component, createRef, renderToString } from '@wordpress/element'; -import { Button, Icon, Tooltip } from '@wordpress/components'; +import { Button, Icon } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { withSelect, withDispatch } from '@wordpress/data'; import { compose } from '@wordpress/compose'; @@ -236,25 +236,23 @@ class AMPPreview extends Component { return ( isEnabled && ! errorMessages.length && ! isStandardMode && ( - - - + ) ); } diff --git a/contributing.md b/contributing.md index 53536bc54a7..542050a67ea 100644 --- a/contributing.md +++ b/contributing.md @@ -1,46 +1 @@ -# AMP contributing guide - -We'd love to accept your patches and contributions to this project and hope you'll become an ongoing participant in our open source community but we also welcome one-off contributions for the issues you're particularly passionate about. - -**How would you like to help?** - -* [Report a bug](#report-a-bug) -* [Make a suggestion](#make-a-suggestion) -* [Contribute code](#product-and-code-contributions) - -## Report a bug - -[File an issue](https://github.com/ampproject/amp-wp/issues/new?template=bug_report.md) if you find a bug in the AMP Plugin. Providing details for each section predefined in the issue template is much appreciated! - -Project maintainers are regularly monitoring issues and aim to fix open bugs quickly. - -## Make a suggestion - -[File a "feature request" issue](https://github.com/ampproject/amp-wp/issues/new?template=feature_request.md) if you have a suggestion for a way to improve the AMP Plugin, or a request for a new feature. - -## Product and code contributions - -We'd love to have your help contributing code and features to the AMP Plugin! Head to the [Project Management guidelines](https://github.com/ampproject/amp-wp/blob/develop/contributing/project-management.md) as well as [Engineering guidelines](https://github.com/ampproject/amp-wp/blob/develop/contributing/engineering.md) for details on the process you can use to contribute. - -## Contributor license agreement - -Contributions to this project must be accompanied by a Contributor License Agreement. You (or your employer) retain the copyright to your contribution; this simply gives us permission to use and redistribute your contributions as part of the project. Head over to https://cla.developers.google.com/ to see your current agreements on file or to sign a new one. - -You generally only need to submit a CLA once, so if you've already submitted one (even if it was for a different project), you probably don't need to do it again. - -## Contributors list policy - -The list of contributors who are featured on the WordPress.org plugin directory are subject to change over time. The organizations and individuals who contribute significantly and consistently (e.g. 3-month period) to the project are eligible to be listed. Those listed should generally be considered as those who take responsibility for the project (i.e. owners). Note that contributions include more than just code, though contributors who commit are [most visible](https://github.com/ampproject/amp-wp/graphs/contributors). The sort order of the contributors list should generally follow the sort order of the GitHub contributors page, though again, this order does not consider work in issues and the support forum, so it cannot be relied on solely. - -## Community guidelines - -This project follows -[Google's Open Source Community Guidelines](https://opensource.google.com/conduct/). - -## Security disclosures - -The AMP Project accepts responsible security disclosures through the [Google Application Security program](https://www.google.com/about/appsecurity/). - -## Code of conduct - -In addition to the Community Guidelines, this project follows an explicit [Code of Conduct](https://github.com/ampproject/amp-wp/blob/develop/code_of_conduct.md). +This document has moved to the [Contributing](https://github.com/ampproject/amp-wp/wiki/Contributing) page on the wiki. diff --git a/contributing/engineering.md b/contributing/engineering.md deleted file mode 100644 index 54d4c73f3d3..00000000000 --- a/contributing/engineering.md +++ /dev/null @@ -1,376 +0,0 @@ -# Engineering guidelines - -## Getting started - -### Requirements - -To contribute to this plugin, you need the following tools installed on your computer: - -* [Composer](https://getcomposer.org/) - to install PHP dependencies. -* [Node.js](https://nodejs.org/en/) - to install JavaScript dependencies. -* [WordPress](https://wordpress.org/download/) - to run the actual plugin. - -You should be running a Node version matching the [current active LTS release](https://github.com/nodejs/Release#release-schedule) or newer for this plugin to work correctly. You can check your Node.js version by typing node -v in the Terminal prompt. - -If you have an incompatible version of Node in your development environment, you can use [nvm](https://github.com/creationix/nvm) to change node versions on the command line: - -```bash -nvm install -``` - -## Local environment - -Since you need a WordPress environment to run the plugin in, the quickest way to get up and running is to use the provided Docker setup. Install [Docker](https://www.docker.com/products/docker-desktop) and [Docker Compose](https://docs.docker.com/compose/install/) by following the instructions on their website. - -You can then clone this project somewhere on your computer: - -```bash -git clone git@github.com:ampproject/amp-wp.git amp -cd amp -``` - -After that, run a script to set up the local environment. It will automatically verify whether Docker, Composer and Node.js are configured properly and start the local WordPress instance. You may need to run this script multiple times if prompted. - -```bash -./bin/local-env/start.sh -``` - -If everything was successful, you'll see this on your screen: - -``` -Welcome to... - -MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM -MMMMMMMMMMMMMMMMWNXK0OOkkkkOO0KXNWMMMMMMMMMMMMMMMM -MMMMMMMMMMMMWX0kdlc:::::::::::ccodk0NWMMMMMMMMMMMM -MMMMMMMMMWXOdl::::::::::::::::::::::lx0NMMMMMMMMMM -MMMMMMMWKxl::::::::::::::::::::::::::::oOXWMMMMMMM -MMMMMMXkl:::::::::::::::::col::::::::::::oONMMMMMM -MMMMW0o:::::::::::::::::ox0Xk:::::::::::::cxXWMMMM -MMMW0l:::::::::::::::::dKWWXd:::::::::::::::dXMMMM -MMW0l::::::::::::::::cxXWMM0l::::::::::::::::dXMMM -MMXd::::::::::::::::ckNMMMWkc::::::::::::::::ckWMM -MWOc:::::::::::::::lONMMMMNkooool:::::::::::::oXMM -MWk:::::::::::::::l0WMMMMMMWNWNNOc::::::::::::l0MM -MNx::::::::::::::oKWMMMMMMMMMMW0l:::::::::::::cOWM -MNx:::::::::::::oKWWWMMMMMMMMNOl::::::::::::::c0MM -MWOc::::::::::::cddddxKWMMMMNkc:::::::::::::::oKMM -MMXd:::::::::::::::::l0MMMMXdc:::::::::::::::ckWMM -MMW0l::::::::::::::::dXMWWKd:::::::::::::::::oXMMM -MMMWOl:::::::::::::::kWW0xo:::::::::::::::::oKWMMM -MMMMW0l:::::::::::::l0NOl::::::::::::::::::dKWMMMM -MMMMMWKdc:::::::::::cooc:::::::::::::::::lkNMMMMMM -MMMMMMMN0dc::::::::::::::::::::::::::::lxKWMMMMMMM -MMMMMMMMMWKxoc::::::::::::::::::::::coOXWMMMMMMMMM -MMMMMMMMMMMWNKkdoc:::::::::::::cloxOKWMMMMMMMMMMMM -MMMMMMMMMMMMMMMWNX0OkkxxxxxxkO0KXWWMMMMMMMMMMMMMMM -MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM -``` - -The WordPress installation should be available at http://localhost:8890 (**Username**: admin, **Password**: password). - -To later turn off the local environment, you can run: - -```bash -npm run env:stop -``` - -To bring it back later, run: - -```bash -npm run env:start -``` - -Also, if you need to reset the local environment's database, you can run: - -```bash -npm run env:reset-site -``` - -### Custom environment - -Alternatively, you can use your own local WordPress environment and clone this repository right into your `wp-content/plugins` directory. - -```bash -cd wp-content/plugins && git clone git@github.com:ampproject/amp-wp.git amp -``` - -Then install the packages: - -```bash -composer install -npm install -``` - -And lastly, do a build of the JavaScript: - -```bash -npm run build:js -``` - -Lastly, to get the plugin running in your WordPress install, run `composer install` and then activate the plugin via the WordPress dashboard or `wp plugin activate amp`. - -## Developing the plugin - -Whether you use the pre-existing local environment or a custom one, any PHP code changes will be directly visible during development. - -However, for JavaScript this involves a build process. To watch for any JavaScript file changes and re-build it when needed, you can run the following command: - -```bash -npm run dev -``` - -This way you will get a development build of the JavaScript, which makes debugging easier. - -To get a production build, run: - -```bash -npm run build:js -``` - -### Branches - -The branching strategy follows the [GitFlow schema](https://datasift.github.io/gitflow/IntroducingGitFlow.html); make sure to familiarize yourself with it. - -All branches are named with with the following pattern: `{type}`/`{issue_id}`-`{short_description}` - -* `{type}` = issue Type label -* `{issue_id}` = issue ID -* `{short_description}` = short description of the PR - -To include your changes in the next patch release (e.g. `1.0.x`), please base your branch off of the current release branch (e.g. `1.0`) and open your pull request back to that branch. If you open your pull request with the `develop` branch then it will be by default included in the next minor version (e.g. `1.x`). - -### Code reviews - -All submissions, including submissions by project members, require review. We use GitHub pull requests for this purpose. Consult [GitHub Help](https://help.github.com/articles/about-pull-requests/) for more information on using pull requests. - -### Coding standards - -All contributions to this project will be checked against [WordPress-Coding-Standards](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards) with PHPCS, and for JavaScript linting is done with ESLint. - -To verify your code meets the requirements, you can run `npm run lint`. - -You can also install a `pre-commit` hook by running `bash vendor/xwp/wp-dev-lib/scripts/install-pre-commit-hook.sh`. This way, your code will be checked automatically before committing any changes. - -### Tests - -#### PHP Unit Tests - -The AMP plugin uses the [PHPUnit](https://phpunit.de/) testing framework to write unit and integration tests for the PHP part. - -To run the full test suite, you can use the following command: - -```bash -npm run test:php -``` - -You can also just run test for a specific function or class by using something like this: - -```bash -npm run test:php -- --filter=AMP_Theme_Support -``` - -See `npm run test:php:help` to see all the possible options. - -#### JavaScript Unit Tests - -[Jest](https://jestjs.io/) is used as the JavaScript unit testing framework. - -To run the full test suite, you can use the following command: - -```bash -npm run test:js -``` - -You can also watch for any file changes and only run tests that failed or have been modified: - -```bash -npm run test:js:watch -``` - -See `npm run test:js:help` to get a list of additional options that can be passed to the test runner. - -#### End-to-End Tests - -This project leverages the local Docker-based environment to facilitate end-to-end (e2e) testing using Puppeteer. - -To run the full test suite, you can use the following command: - -```bash -npm run test:e2e -``` - -You can also watch for any file changes and only run tests that failed or have been modified: - -```bash -npm run test:e2e:watch -``` - -Not using the built-in local environment? You can also pass any other URL to run the tests against. Example: - -```bash -npm run test:e2e -- --wordpress-base-url=https://my-amp-dev-site.local -``` - -For debugging purposes, you can also run the E2E tests in non-headless mode: - -```bash -npm run test:e2e:interactive -``` - -Note that this will also slow down all interactions during tests by 80ms. You can control these values individually too: - -```bash -PUPPETEER_HEADLESS=false npm run test:e2e # Interactive mode, normal speed. -PUPPETEER_SLOWMO=200 npm run test:e2e # Headless mode, slowed down by 200ms. -``` - -Sometimes one might want to test additional scenarios that aren't possible in a WordPress installation out of the box. That's why the test setup allows for for adding some utility plugins that can be activated during E2E tests. - -For example, such a plugin could create a custom post type and the accompanying E2E test would verify that block validation errors are shown for this custom post type too. - -These plugins can be added to `tests/e2e/plugins` and then activated via the WordPress admin. - -#### Testing media and embed support - -The following script creates a post in order to test support for WordPress media and embeds. -To run it: -1. `ssh` into an environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV) -2. `cd` to the root of this plugin -3. run `wp eval-file bin/create-embed-test-post.php` -4. go to the URL that is output in the command line - -#### Testing widgets support - -The following script adds an instance of every default WordPress widget to the first registered sidebar. -To run it: -1. `ssh` into an environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV) -2. `cd` to the root of this plugin -3. run `wp eval-file bin/add-test-widgets-to-sidebar.php` -4. There will be a message indicating which sidebar has the widgets. Please visit a page with that sidebar. - -#### Testing comments support - -The following script creates a post with comments in order to test support for WordPress comments. -To run it: -1. `ssh` into an environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV) -2. `cd` to the root of this plugin -3. run `wp eval-file bin/create-comments-on-test-post.php` -4. go to the URL that is output in the command line - -#### Testing Gutenberg block support - -The following script creates a post with all core Gutenberg blocks. To run it: -1. `ssh` into an environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV) -2. `cd` to the root of this plugin -3. run `bash bin/create-gutenberge-test-post.sh` -4. go to the URL that is output in the command line - -## Updating allowed tags and attributes - -The file `class-amp-allowed-tags-generated.php` has the AMP specification's allowed tags and attributes. It's used in sanitization. - -To update that file, perform the following steps: - -1. `cd` to the root of this plugin -2. Run `./bin/amphtml-update.sh` (or `lando ssh -c './bin/amphtml-update.sh'` if using Lando). -3. Review the diff. -4. Update tests based on changes to the spec. -5. Commit changes. - -This script is intended for a Linux environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV) or [Lando wordpressdev](https://github.com/felixarntz/wordpressdev). - - -## Creating a plugin build - -To create a build of the plugin for installing in WordPress as a ZIP package, run: - -```bash -npm run build -``` - -This will create an `amp.zip` in the plugin directory which you can install. The contents of this ZIP are also located in the `build` directory which you can `rsync` somewhere as well if needed. - -## Creating a prerelease - -1. Create changelog draft on [Wiki page](https://github.com/ampproject/amp-wp/wiki/Release-Changelog-Draft). -1. Check out the branch intended for release (`develop` for major, `x.y` for minor) and pull latest commits. -1. Bump plugin versions in `amp.php` (×2: the metadata block in the header and also the `AMP__VERSION` constant). -1. Do `npm install && composer selfupdate && composer install -o`. -1. Do `npm run build` and install the `amp.zip` onto a normal WordPress install running a stable release build; do smoke test to ensure it works. -1. [Draft new release](https://github.com/ampproject/amp-wp/releases/new) on GitHub targeting the required branch (`develop` for major, `x.y` for minor). - 1. Use the new plugin version as the tag (e.g. `1.2-beta3` or `1.2.1-RC1`) - 1. Use new version as the title, followed by some highlight tagline of the release. - 1. Attach the `amp.zip` build to the release. - 1. Add changelog entry to the release, link to compare view comparing previous release, and link to milestone. - 1. Make sure “Pre-release” is checked. -1. Publish GitHub release. -1. Create built release tag (from the just-created `build` directory): - 1. Do `git fetch origin --tags && ./bin/tag-built.sh` - 1. Add link from release notes. -1. Make announcements on Twitter and the #amp-wp channel on AMP Slack, linking to release post or GitHub release. -1. Bump version in release branch, e.g. `…-alpha` to `…-beta1` and `…-beta2` to `…-RC1` - -## Creating a stable release - -Contributors who want to make a new release, follow these steps: - -1. Create changelog draft on [Wiki page](https://github.com/ampproject/amp-wp/wiki/Release-Changelog-Draft). - 1. Gather props list of the entire release, including contributors of code, design, testing, project management, etc. -1. Update readme including the description, contributors, and screenshots (as needed). -1. For major release, draft blog post about the new release. -1. For minor releases, make sure all merged commits in `develop` have been also merged onto release branch. -1. Check out the branch intended for release (`develop` for major, `x.y` for minor) and pull latest commits. -1. Do `npm install && composer selfupdate && composer install -o`. -1. Bump plugin versions in `amp.php` (×2: the metadata block in the header and also the `AMP__VERSION` constant). Verify via `npx grunt shell:verify_matching_versions`. Ensure patch version number is supplied for major releases, so `1.2-RC1` should bump to `1.2.0`. -1. Ensure "Tested Up To" is updated to current WordPress version. -1. Do `npm run build` and install the `amp.zip` onto a normal WordPress install running a stable release build; do smoke test to ensure it works. -1. Optionally do sanity check by comparing the `build` directory with the previously-deployed plugin on WordPress.org for example: `svn export https://plugins.svn.wordpress.org/amp/trunk /tmp/amp-trunk; diff /tmp/amp-trunk/ ./build/` (instead of straight `diff`, it's best to use a GUI like `idea diff`, `phpstorm diff`, or `opendiff`). -1. [Draft new release](https://github.com/ampproject/amp-wp/releases/new) on GitHub targeting the required branch (`develop` for major, `x.y` for minor): - 1. Use the new plugin version as the tag (e.g. `1.2.0` or `1.2.1`) - 1. Attach the `amp.zip` build to the release. - 1. Add changelog entry to the release, link to compare view comparing previous release, and link to milestone. -1. Publish GitHub release. -1. Run `npm run deploy` to commit the plugin to WordPress.org. -1. Confirm the release is available on WordPress.org; try installing it on a WordPress install and confirm it works. -1. Create built release tag (from the just-created `build` directory): - 1. Do `git fetch origin --tags && ./bin/tag-built.sh` - 1. Add link from release notes. -1. For new major releases, create a release branch from the tag. Patch versions are made from the release branch. -1. For minor releases, bump `Stable tag` in the `readme.txt`/`readme.md` in `develop`. Cherry-pick other changes as necessary. -1. Merge release tag into `master`. -1. Close the GitHub milestone (and project). -1. Publish release blog post (if applicable), including link to GitHub release. -1. Make announcements on Twitter and the #amp-wp channel on AMP Slack, linking to release post or GitHub release. -1. Bump version in release branch. After major release (e.g. `1.2.0`), bump to `1.3.0-alpha` on `develop`; after minor release (e.g. `1.2.1`) bump version to `1.2.2-alpha` on the release branch. - -## Changelog - -Release changelogs are created by an automation script that accumulates changelog messages from issues associated with a given milestone. - -### Changelog messages - -* Changelog messages are added in the PR-related issue, within its reserved section, which is pre-populated from the issue template. -* Changelog messages start with a verb in its imperative form (e.g. “Fix bug xyz”), preferably one of the following words: - * Add (for features) - * Introduce (for features) - * Enhance (for enhancements) - * Improve (for enhancements) - * Change (for misc changes) - * Update (for misc changes) - * Modify (for misc changes) - * Remove (for removal) - * Fix (for bug fixes) - * N/A (skip changelog message) - -### Changelog format - -* The changelog messages are categorized as follows: - * Added - * Enhanced - * Changed - * Fixed -* Changelog messages are automatically assigned to one of the defined categories based on the first word the message starts with. Default: “Changed”. -* Changelogs with the message “N/A” are skipped. - -Maintainers must ensure that changelog messages are clear and follow the formatting guidelines. diff --git a/contributing/project-management.md b/contributing/project-management.md deleted file mode 100644 index ba855941e23..00000000000 --- a/contributing/project-management.md +++ /dev/null @@ -1,109 +0,0 @@ -# Project management guidelines - -|IMPORTANT: The project management guidelines are only applicable to AMP Stories at this stage.| -| :--- | - -# Project boards - -In addition to [Milestones](https://github.com/ampproject/amp-wp/milestones), which are used to manage releases, project boards are used to manage issue statuses. - -### Definition project board - -The [Definition](https://github.com/ampproject/amp-wp/projects/17) project board covers the pipeline which issues go through in preparation for execution. The board contains the following columns: - -* Revisit Later -* Prioritization -* Acceptance Criteria -* Implementation Brief / Estimate -* Implementation Brief Review -* Estimate - -### Execution project board - -The [Execution](https://github.com/ampproject/amp-wp/projects/16) project board covers the execution pipeline which issues go through for implementation. The board contains the following columns: - -* Blocked -* Backlog -* To Do -* In Progress -* Code Review -* QA -* Demo -* Approval -* Done - -## Labels - -The labels below are utilized to categorize issues: - -* Type: `{type}` = the issue type (ex. `Type: Bug`, `Type: Feature`, `Type: Support`) -* P`{priority}` = the priority of the task (ex. `P1`, `P2`, `P3`) -* Size: `{size`} = the issue size (ex. `S`, `M`, `L`) -* Sprint: `{sprint_number`} = the sprint associated to the issue (ex. `Sprint: 1`, `Sprint: 2`, `Sprint: 3`) - -## Life of an issue - -|IMPORTANT: We use GitHub issues to track all task statuses, therefore PRs should **only** be associated with an issue, **not** assigned a label, project, and/or milestone.| -| :--- | - -### Triage - -The [GitHub issues](https://github.com/ampproject/amp-wp/issues) view serves as the “Awaiting Triage” backlog. - -1. An issue is created. -1. The issue “Type” label is assigned. - -### Issue cycle by type - -#### Type: `Bug`, `Enhancement`, `Feature` - -##### Definition - -|
Step |
Task |                                   
Role | -| :--- | :--- | :--- | -| 1. | An issue requiring work is added to the [Definition](https://github.com/ampproject/amp-wp/projects/17) project board (automatically added to Prioritization column). | `Product Manager` `Program Manager` -| 2. | The issue is assigned a “Priority” and moved to the “Acceptance Criteria” column. | `Product Manager` `Program Manager` -| 3. | “Acceptance Criteria” are added to the issue description, and the issue is moved to the “Implementation Brief” column. | `Product Manager` `Product Owner` `Lead Engineer` -| 4. | “Implementation Brief” is added to the issue description, and the issue is moved to the “Implementation Brief Review” column. | `Engineer` -| 5. | The “Implementation Brief” is reviewed and the issue is moved to the “Estimate” column upon approval. The issue is moved back to the “Implementation Brief” column if changes in the “Implementation Brief” description are requested. | `Lead Engineer` -| 6. | The issue is estimated using T-Shirt sizing and moved to the [Execution](https://github.com/ampproject/amp-wp/projects/16) project board (automatically added to the Backlog). | `Project Manager` `Engineer` - -##### Execution -|
Step |
Task |                                   
Role | -| :--- | :--- | :--- | -| 1. | An issue requiring work is added to the [Execution](https://github.com/ampproject/amp-wp/projects/16) project board (automatically added to the Backlog) after going through the [Definition](https://github.com/ampproject/amp-wp/projects/17) project board pipeline. | `Product Manager` `Program Manager` -| 2. | The issue is assigned a “[Milestone](https://github.com/ampproject/amp-wp/milestones)” and “Sprint” label. | `Product Manager` `Program Manager` -| 3. | The issue is moved to the “To Do” column if it is assigned to the current sprint. | `Project Manager` -| 4. | The issue is assigned (or may be self-assigned) to an engineer. | `Project Manager` `Engineer` -| 5. | The issue is moved to the “In Progress” column when development starts. A PR is created, following the [Branching Strategy](https://github.com/ampproject/amp-wp/contributing/engineering.md#branches). The PR must contain details for each section predefined in the PR template, with a reference to the associated issue. **IMPORTANT:** do not add [GitHub keywords](https://help.github.com/en/articles/closing-issues-using-keywords) which would automatically close an issue once the PR is merged. | `Engineer` -| 6. | The “[Changelog Message](https://github.com/ampproject/amp-wp/contributing/engineering.md#changelog)” is added to the relevant section of the issue description once development is completed. | `Engineer` -| 7. | The “QA Testing Instructions“ are added to the relevant section of the issue description and the issue is moved to the “Code Review” column. | `Engineer` -| 8. | The code review is done in the referred PR and the issue is moved to the “QA” column once the review is completed and the PR is approved, merged and deployed to the QA environment. The reviewer must ensure that the “Acceptance Criteria” match the implementation and that the “QA Testing Instructions“ has been added to the issue before moving it to QA. | `Engineer` -| 9. | The issue is moved to the “Demo” column once QA is passed or moved back to the “To Do” column if changes are required, in which case the cycle from the “To Do” column onwards is repeated. | `QA Specialist` -| 9. | A video or screenshots demoing the implementation are added to the relevant section of the issue description. | `Engineer` -| 10. | The issue is moved to the “Approval” column once the demo is added. | `Engineer` -| 11. | The issue goes through a final review and moved to the “Done” once approved or moved back to the “To Do” column if changes are required, in which case the cycle from the “To Do” column onwards is repeated. | `Product Manager` -| 12. | The issue is closed. | `Product Manager` - -#### Type: `Task` - -##### Definition - -|
Step |
Task |                                   
Role | -| :--- | :--- | :--- | -| 1. | An issue requiring work is added to the [Definition](https://github.com/ampproject/amp-wp/projects/17) project board (automatically added to Prioritization column). | `Product Manager` `Program Manager` -| 2. | The issue is assigned a “Priority” and moved to the “Acceptance Criteria” column. | `Product Manager` `Program Manager` -| 3. | “Acceptance Criteria” are added to the issue description, and the issue is moved to the “Estimate" column. | `Product Manager` `Product Owner` -| 4. | The issue is estimated using T-Shirt sizing and moved to the [Execution](https://github.com/ampproject/amp-wp/projects/16) project board (automatically added to the Backlog). | `Task specific` - -##### Execution -|
Step |
Task |                                   
Role | -| :--- | :--- | :--- | -| 1. | An issue requiring work is added to the [Execution](https://github.com/ampproject/amp-wp/projects/16) project board (automatically added to the Backlog) after going through the [Definition](https://github.com/ampproject/amp-wp/projects/17) project board pipeline. | `Product Manager` `Program Manager` -| 2. | The issue is assigned a “Sprint” label. | `Product Manager` `Program Manager` -| 3. | The issue is moved to the “To Do” column if it is assigned to the current sprint. | `Project Manager` -| 4. | The issue is assigned (or may be self-assigned) | `Project Manager` `Assignee` -| 5. | The issue is moved to the “In progress” column when work starts. | `Assignee` -| 8. | The issue is moved to the “Approval” column the work is done. | `Assignee` -| 9. | The issue goes through a final review and moved to the “Done” once approved or moved back to the “To Do” column if changes are required, in which case the cycle from the “To Do” column onwards is repeated. | `Product Manager` -| 10. | The issue is closed. | `Product Manager` diff --git a/includes/admin/functions.php b/includes/admin/functions.php index ace4f332343..ba9ad3b7c55 100644 --- a/includes/admin/functions.php +++ b/includes/admin/functions.php @@ -6,6 +6,7 @@ */ use AmpProject\AmpWP\Admin\SiteHealth; +use AmpProject\AmpWP\Option; /** * Obsolete constant for flagging when Customizer is opened for AMP. @@ -22,7 +23,7 @@ * And this does not need to toggle between the AMP and normal display. */ function amp_init_customizer() { - if ( AMP_Theme_Support::READER_MODE_SLUG !== AMP_Options_Manager::get_option( 'theme_support' ) ) { + if ( AMP_Theme_Support::READER_MODE_SLUG !== AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) ) { return; } diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 5a043682087..b7d1212d7d5 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -5,6 +5,7 @@ * @package AMP */ +use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Services; /** @@ -118,20 +119,20 @@ function amp_init() { /* * Broadcast plugin updates. - * Note that AMP_Options_Manager::get_option( 'version', '0.0' ) cannot be used because + * Note that AMP_Options_Manager::get_option( Option::VERSION, '0.0' ) cannot be used because * version was new option added, and in that case default would never be used for a site * upgrading from a version prior to 1.0. So this is why get_option() is currently used. */ $options = get_option( AMP_Options_Manager::OPTION_NAME, [] ); - $old_version = isset( $options['version'] ) ? $options['version'] : '0.0'; - if ( AMP__VERSION !== $old_version ) { + $old_version = isset( $options[ Option::VERSION ] ) ? $options[ Option::VERSION ] : '0.0'; + if ( AMP__VERSION !== $old_version && is_admin() && current_user_can( 'manage_options' ) ) { /** * Triggers when after amp_init when the plugin version has updated. * * @param string $old_version Old version. */ do_action( 'amp_plugin_update', $old_version ); - AMP_Options_Manager::update_option( 'version', AMP__VERSION ); + AMP_Options_Manager::update_option( Option::VERSION, AMP__VERSION ); } } @@ -479,8 +480,10 @@ function amp_remove_endpoint( $url ) { * If there are known validation errors for the current URL then do not output anything. * * @since 1.0 + * @global WP_Query $wp_query */ function amp_add_amphtml_link() { + global $wp_query; /** * Filters whether to show the amphtml link on the frontend. @@ -499,7 +502,7 @@ function amp_add_amphtml_link() { if ( AMP_Theme_Support::is_paired_available() ) { $amp_url = add_query_arg( amp_get_slug(), '', $current_url ); } - } elseif ( is_singular() && post_supports_amp( get_post( get_queried_object_id() ) ) ) { + } elseif ( $wp_query instanceof WP_Query && ( $wp_query->is_singular() || $wp_query->is_posts_page ) && post_supports_amp( get_post( get_queried_object_id() ) ) ) { $amp_url = amp_get_permalink( get_queried_object_id() ); } @@ -566,41 +569,62 @@ function post_supports_amp( $post ) { function is_amp_endpoint() { global $pagenow, $wp_query; - if ( is_admin() || is_embed() || is_feed() || ( defined( 'REST_REQUEST' ) && REST_REQUEST ) || in_array( $pagenow, [ 'wp-login.php', 'wp-signup.php', 'wp-activate.php' ], true ) ) { + // Short-circuit for admin requests or requests to non-frontend pages. + if ( is_admin() || in_array( $pagenow, [ 'wp-login.php', 'wp-signup.php', 'wp-activate.php' ], true ) ) { return false; } - // Always return false when requesting service worker. - if ( class_exists( 'WP_Service_Workers' ) && ! empty( $wp_query ) && defined( 'WP_Service_Workers::QUERY_VAR' ) && $wp_query->get( WP_Service_Workers::QUERY_VAR ) ) { + $warned = false; + $error_message = sprintf( + /* translators: %1$s: is_amp_endpoint(), %2$s: the current action, %3$s: the wp action, %4$s: the WP_Query class, %5$s: the amp_skip_post() function */ + __( '%1$s was called too early and so it will not work properly. WordPress is currently doing the "%2$s" action. Calling this function before the "%3$s" action means it will not have access to %4$s and the queried object to determine if it is an AMP response, thus neither the "%5$s" filter nor the AMP enabled toggle will be considered.', 'amp' ), + __FUNCTION__ . '()', + current_action(), + 'wp', + 'WP_Query', + 'amp_skip_post()' + ); + + // Make sure the parse_request action has triggered before trying to read from the REST_REQUEST constant, which is set during rest_api_loaded(). + if ( ! did_action( 'parse_request' ) ) { + _doing_it_wrong( __FUNCTION__, esc_html( $error_message ), '1.6.0' ); + $warned = true; + } elseif ( defined( 'REST_REQUEST' ) && REST_REQUEST ) { return false; } - $did_parse_query = did_action( 'parse_query' ); + // Make sure that the parse_query action has triggered, as this is required to initially populate the global WP_Query. + if ( ! $warned && ! ( $wp_query instanceof WP_Query || did_action( 'parse_query' ) ) ) { + _doing_it_wrong( __FUNCTION__, esc_html( $error_message ), '0.4.2' ); + $warned = true; + } - if ( ! $did_parse_query ) { - _doing_it_wrong( - __FUNCTION__, - sprintf( - /* translators: 1: is_amp_endpoint(), 2: parse_query */ - esc_html__( '%1$s was called before the %2$s hook was called.', 'amp' ), - 'is_amp_endpoint()', - 'parse_query' - ), - '0.4.2' - ); + // Always return false when requesting the service worker. + // Note this is no longer required because AMP_Theme_Support::prepare_response() will abort for non-HTML responses. + if ( class_exists( 'WP_Service_Workers' ) && $wp_query instanceof WP_Query && defined( 'WP_Service_Workers::QUERY_VAR' ) && $wp_query->get( WP_Service_Workers::QUERY_VAR ) ) { + return false; } - if ( empty( $wp_query ) || ! ( $wp_query instanceof WP_Query ) ) { - _doing_it_wrong( - __FUNCTION__, - sprintf( - /* translators: 1: is_amp_endpoint(), 2: WP_Query */ - esc_html__( '%1$s was called before the %2$s was instantiated.', 'amp' ), - 'is_amp_endpoint()', - 'WP_Query' - ), - '1.1' - ); + // Short-circuit queries that can never have AMP responses (e.g. post embeds and feeds). + // Note that these conditionals only require the parse_query action to have been run. They don't depend on the wp action having been fired. + if ( + $wp_query instanceof WP_Query + && + ( + $wp_query->is_embed() + || + $wp_query->is_feed() + || + $wp_query->is_comment_feed() + || + $wp_query->is_trackback() + || + $wp_query->is_robots() + || + ( method_exists( $wp_query, 'is_favicon' ) && $wp_query->is_favicon() ) + ) + ) { + return false; } /* @@ -623,34 +647,29 @@ function is_amp_endpoint() { ) ); - if ( ! current_theme_supports( AMP_Theme_Support::SLUG ) ) { - return $has_amp_query_var; - } - // When there is no query var and AMP is not canonical (AMP-first), then this is definitely not an AMP endpoint. if ( ! $has_amp_query_var && ! amp_is_canonical() ) { return false; } - if ( ! did_action( 'wp' ) ) { - _doing_it_wrong( - __FUNCTION__, - sprintf( - /* translators: 1: is_amp_endpoint(). 2: wp. 3: amp_skip_post */ - esc_html__( '%1$s was called before the %2$s action which means it will not have access to the queried object to determine if it is an AMP response, thus neither the %3$s filter nor the AMP enabled publish metabox toggle will be considered.', 'amp' ), - 'is_amp_endpoint()', - 'wp', - 'amp_skip_post' - ), - '1.0.2' - ); - $supported = true; + if ( did_action( 'wp' ) && $wp_query instanceof WP_Query ) { + if ( current_theme_supports( AMP_Theme_Support::SLUG ) ) { + $availability = AMP_Theme_Support::get_template_availability( $wp_query ); + return $availability['supported']; + } else { + $queried_object = get_queried_object(); + return $queried_object instanceof WP_Post && ( $wp_query->is_singular() || $wp_query->is_posts_page ) && post_supports_amp( $queried_object ); + } } else { - $availability = AMP_Theme_Support::get_template_availability(); - $supported = $availability['supported']; + // If WP_Query was not available yet, then we will just assume the query is supported since at this point we do + // know either that the site is in Standard mode or the URL was requested with the AMP query var. This can still + // produce an undesired result when a Standard mode site has a post that opts out of AMP, but this issue will + // have been flagged via _doing_it_wrong() above. + if ( ! $warned ) { + _doing_it_wrong( __FUNCTION__, esc_html( $error_message ), '1.0.2' ); + } + return amp_is_canonical() || $has_amp_query_var; } - - return amp_is_canonical() ? $supported : ( $has_amp_query_var && $supported ); } /** @@ -948,7 +967,7 @@ function amp_filter_font_style_loader_tag_with_crossorigin_anonymous( $tag, $han * @return array Analytics. */ function amp_get_analytics( $analytics = [] ) { - $analytics_entries = AMP_Options_Manager::get_option( 'analytics', [] ); + $analytics_entries = AMP_Options_Manager::get_option( Option::ANALYTICS, [] ); /** * Add amp-analytics tags. @@ -1220,6 +1239,7 @@ function amp_get_content_sanitizers( $post = null ) { 'AMP_Style_Sanitizer' => [], 'AMP_Meta_Sanitizer' => [], 'AMP_Layout_Sanitizer' => [], + 'AMP_Accessibility_Sanitizer' => [], 'AMP_Tag_And_Attribute_Sanitizer' => [], // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch. ]; diff --git a/includes/class-amp-post-type-support.php b/includes/class-amp-post-type-support.php index d307a3e7559..e1129a6a285 100644 --- a/includes/class-amp-post-type-support.php +++ b/includes/class-amp-post-type-support.php @@ -6,6 +6,8 @@ * @since 0.6 */ +use AmpProject\AmpWP\Option; + /** * Class AMP_Post_Type_Support. */ @@ -56,10 +58,10 @@ public static function get_eligible_post_types() { * @since 0.6 */ public static function add_post_type_support() { - if ( current_theme_supports( AMP_Theme_Support::SLUG ) && AMP_Options_Manager::get_option( 'all_templates_supported' ) ) { + if ( current_theme_supports( AMP_Theme_Support::SLUG ) && AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ) { $post_types = self::get_eligible_post_types(); } else { - $post_types = AMP_Options_Manager::get_option( 'supported_post_types', [] ); + $post_types = AMP_Options_Manager::get_option( Option::SUPPORTED_POST_TYPES, [] ); } foreach ( $post_types as $post_type ) { add_post_type_support( $post_type, self::SLUG ); diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 47170285c1b..40286768734 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -6,6 +6,7 @@ */ use AmpProject\Amp; +use AmpProject\AmpWP\Option; use AmpProject\AmpWP\RemoteRequest\CachedRemoteGetRequest; use AmpProject\AmpWP\ConfigurationArgument; use AmpProject\AmpWP\Transformer; @@ -253,7 +254,7 @@ public static function read_theme_support() { self::$support_added_via_theme = null; self::$support_added_via_option = null; - $theme_support_option = AMP_Options_Manager::get_option( 'theme_support' ); + $theme_support_option = AMP_Options_Manager::get_option( Option::THEME_SUPPORT ); if ( current_theme_supports( self::SLUG ) ) { $args = self::get_theme_support_args(); @@ -282,7 +283,7 @@ public static function read_theme_support() { /* translators: 1: available_callback. 2: supported_templates */ esc_html__( 'The %1$s is deprecated when adding amp theme support in favor of declaratively setting the %2$s.', 'amp' ), 'available_callback', - 'supported_templates' + Option::SUPPORTED_TEMPLATES // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ), '1.0' ); @@ -376,25 +377,14 @@ public static function finish_init() { add_filter( 'template_include', [ __CLASS__, 'serve_paired_browsing_experience' ] ); } + $is_reader_mode = self::READER_MODE_SLUG === self::get_support_mode(); $has_query_var = ( isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended || false !== get_query_var( amp_get_slug(), false ) ); - $is_reader_mode = self::READER_MODE_SLUG === self::get_support_mode(); - if ( - $is_reader_mode - && - $has_query_var - && - ( ! is_singular() || ! post_supports_amp( get_post( get_queried_object_id() ) ) ) - ) { - // Reader mode only supports the singular template (for now) so redirect non-singular queries in reader mode to non-AMP version. - // Also ensure redirecting to non-AMP version when accessing a post which does not support AMP. - // A temporary redirect is used for admin users to allow them to see changes between reader mode and transitional modes. - wp_safe_redirect( amp_remove_endpoint( amp_get_current_url() ), current_user_can( 'manage_options' ) ? 302 : 301 ); - return; - } elseif ( ! is_amp_endpoint() ) { + + if ( ! is_amp_endpoint() ) { /* * Redirect to AMP-less URL if AMP is not available for this URL and yet the query var is present. * Temporary redirect is used for admin users because implied transitional mode and template support can be @@ -402,7 +392,7 @@ public static function finish_init() { * without wrestling with the redirect cache. */ if ( $has_query_var ) { - self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301, true ); + self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } amp_add_frontend_actions(); @@ -443,11 +433,11 @@ static function() { * Ensure that the current AMP location is correct. * * @since 1.0 + * @since 1.6 Removed $exit param. * - * @param bool $exit Whether to exit after redirecting. - * @return bool Whether redirection was done. Naturally this is irrelevant if $exit is true. + * @return bool Whether redirection should have been done. */ - public static function ensure_proper_amp_location( $exit = true ) { + public static function ensure_proper_amp_location() { $has_query_var = false !== get_query_var( amp_get_slug(), false ); // May come from URL param or endpoint slug. $has_url_param = isset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended @@ -460,7 +450,7 @@ public static function ensure_proper_amp_location( $exit = true ) { * to not be hampered by browser remembering permanent redirects and preventing test. */ if ( $has_query_var || $has_url_param ) { - return self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301, $exit ); + return self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } } elseif ( self::READER_MODE_SLUG === self::get_support_mode() && is_singular() ) { // Prevent infinite URL space under /amp/ endpoint. @@ -468,9 +458,10 @@ public static function ensure_proper_amp_location( $exit = true ) { $path_args = []; wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { - wp_safe_redirect( amp_get_permalink( get_queried_object_id() ), 301 ); - if ( $exit ) { + if ( wp_safe_redirect( amp_get_permalink( get_queried_object_id() ), 301 ) ) { + // @codeCoverageIgnoreStart exit; + // @codeCoverageIgnoreEnd } return true; } @@ -485,13 +476,12 @@ public static function ensure_proper_amp_location( $exit = true ) { $new_url = add_query_arg( amp_get_slug(), '', amp_remove_endpoint( $old_url ) ); if ( $old_url !== $new_url ) { // A temporary redirect is used for admin users to allow them to see changes between reader mode and transitional modes. - wp_safe_redirect( $new_url, current_user_can( 'manage_options' ) ? 302 : 301 ); - // @codeCoverageIgnoreStart - if ( $exit ) { + if ( wp_safe_redirect( $new_url, current_user_can( 'manage_options' ) ? 302 : 301 ) ) { + // @codeCoverageIgnoreStart exit; + // @codeCoverageIgnoreEnd } return true; - // @codeCoverageIgnoreEnd } } return false; @@ -505,25 +495,24 @@ public static function ensure_proper_amp_location( $exit = true ) { * @since 0.7 * @since 1.0 Added $exit param. * @since 1.0 Renamed from redirect_canonical_amp(). + * @since 1.6 Removed $exit param. * - * @param int $status Status code (301 or 302). - * @param bool $exit Whether to exit after redirecting. - * @return bool Whether redirection was done. Naturally this is irrelevant if $exit is true. + * @param int $status Status code (301 or 302). + * @return bool Whether redirection should have be done. */ - public static function redirect_non_amp_url( $status = 302, $exit = true ) { + public static function redirect_non_amp_url( $status = 302 ) { $current_url = amp_get_current_url(); $non_amp_url = amp_remove_endpoint( $current_url ); if ( $non_amp_url === $current_url ) { return false; } - wp_safe_redirect( $non_amp_url, $status ); - // @codeCoverageIgnoreStart - if ( $exit ) { + if ( wp_safe_redirect( $non_amp_url, $status ) ) { + // @codeCoverageIgnoreStart exit; + // @codeCoverageIgnoreEnd } return true; - // @codeCoverageIgnoreEnd } /** @@ -669,7 +658,7 @@ public static function get_template_availability( $query = null ) { $all_templates_supported_by_theme_support = 'all' === $theme_support_args['templates_supported']; } $all_templates_supported = ( - $all_templates_supported_by_theme_support || AMP_Options_Manager::get_option( 'all_templates_supported' ) + $all_templates_supported_by_theme_support || AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ); // Make sure global $wp_query is set in case of conditionals that unfortunately look at global scope. @@ -1008,7 +997,7 @@ public static function get_supportable_templates() { $theme_supported_templates = $theme_support_args['templates_supported']; } - $supported_templates = AMP_Options_Manager::get_option( 'supported_templates' ); + $supported_templates = AMP_Options_Manager::get_option( Option::SUPPORTED_TEMPLATES ); foreach ( $templates as $id => &$template ) { // Capture user-elected support from options. This allows us to preserve the original user selection through programmatic overrides. @@ -1028,7 +1017,7 @@ public static function get_supportable_templates() { // Set supported state from user preference. if ( ! $template['immutable'] ) { - $template['supported'] = AMP_Options_Manager::get_option( 'all_templates_supported' ) || $template['user_supported']; + $template['supported'] = AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) || $template['user_supported']; } } diff --git a/includes/embeds/class-amp-dailymotion-embed.php b/includes/embeds/class-amp-dailymotion-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-dailymotion-embed.php rename to includes/embeds/class-amp-dailymotion-embed-handler.php diff --git a/includes/embeds/class-amp-facebook-embed.php b/includes/embeds/class-amp-facebook-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-facebook-embed.php rename to includes/embeds/class-amp-facebook-embed-handler.php diff --git a/includes/embeds/class-amp-gallery-embed.php b/includes/embeds/class-amp-gallery-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-gallery-embed.php rename to includes/embeds/class-amp-gallery-embed-handler.php diff --git a/includes/embeds/class-amp-instagram-embed.php b/includes/embeds/class-amp-instagram-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-instagram-embed.php rename to includes/embeds/class-amp-instagram-embed-handler.php diff --git a/includes/embeds/class-amp-pinterest-embed.php b/includes/embeds/class-amp-pinterest-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-pinterest-embed.php rename to includes/embeds/class-amp-pinterest-embed-handler.php diff --git a/includes/embeds/class-amp-soundcloud-embed.php b/includes/embeds/class-amp-soundcloud-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-soundcloud-embed.php rename to includes/embeds/class-amp-soundcloud-embed-handler.php diff --git a/includes/embeds/class-amp-vimeo-embed.php b/includes/embeds/class-amp-vimeo-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-vimeo-embed.php rename to includes/embeds/class-amp-vimeo-embed-handler.php diff --git a/includes/embeds/class-amp-vine-embed.php b/includes/embeds/class-amp-vine-embed-handler.php similarity index 100% rename from includes/embeds/class-amp-vine-embed.php rename to includes/embeds/class-amp-vine-embed-handler.php diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php index a654f0b1e8c..28096a8e6b5 100644 --- a/includes/options/class-amp-options-manager.php +++ b/includes/options/class-amp-options-manager.php @@ -5,6 +5,8 @@ * @package AMP */ +use AmpProject\AmpWP\Option; + /** * Class AMP_Options_Manager */ @@ -23,12 +25,12 @@ class AMP_Options_Manager { * @var array */ protected static $defaults = [ - 'theme_support' => AMP_Theme_Support::READER_MODE_SLUG, - 'supported_post_types' => [ 'post' ], - 'analytics' => [], - 'all_templates_supported' => true, - 'supported_templates' => [ 'is_singular' ], - 'version' => AMP__VERSION, + Option::THEME_SUPPORT => AMP_Theme_Support::READER_MODE_SLUG, + Option::SUPPORTED_POST_TYPES => [ 'post' ], + Option::ANALYTICS => [], + Option::ALL_TEMPLATES_SUPPORTED => true, + Option::SUPPORTED_TEMPLATES => [ 'is_singular' ], + Option::VERSION => AMP__VERSION, ]; /** @@ -59,8 +61,8 @@ public static function register_settings() { * @param array $new_options New options. */ public static function maybe_flush_rewrite_rules( $old_options, $new_options ) { - $old_post_types = isset( $old_options['supported_post_types'] ) ? $old_options['supported_post_types'] : []; - $new_post_types = isset( $new_options['supported_post_types'] ) ? $new_options['supported_post_types'] : []; + $old_post_types = isset( $old_options[ Option::SUPPORTED_POST_TYPES ] ) ? $old_options[ Option::SUPPORTED_POST_TYPES ] : []; + $new_post_types = isset( $new_options[ Option::SUPPORTED_POST_TYPES ] ) ? $new_options[ Option::SUPPORTED_POST_TYPES ] : []; sort( $old_post_types ); sort( $new_post_types ); if ( $old_post_types !== $new_post_types ) { @@ -84,17 +86,17 @@ public static function get_options() { $defaults = self::$defaults; if ( current_theme_supports( 'amp' ) ) { - $defaults['theme_support'] = amp_is_canonical() ? AMP_Theme_Support::STANDARD_MODE_SLUG : AMP_Theme_Support::TRANSITIONAL_MODE_SLUG; + $defaults[ Option::THEME_SUPPORT ] = amp_is_canonical() ? AMP_Theme_Support::STANDARD_MODE_SLUG : AMP_Theme_Support::TRANSITIONAL_MODE_SLUG; } $options = array_merge( $defaults, $options ); // Migrate theme support slugs. - if ( 'native' === $options['theme_support'] ) { - $options['theme_support'] = AMP_Theme_Support::STANDARD_MODE_SLUG; - } elseif ( 'paired' === $options['theme_support'] ) { - $options['theme_support'] = AMP_Theme_Support::TRANSITIONAL_MODE_SLUG; - } elseif ( 'disabled' === $options['theme_support'] ) { + if ( 'native' === $options[ Option::THEME_SUPPORT ] ) { + $options[ Option::THEME_SUPPORT ] = AMP_Theme_Support::STANDARD_MODE_SLUG; + } elseif ( 'paired' === $options[ Option::THEME_SUPPORT ] ) { + $options[ Option::THEME_SUPPORT ] = AMP_Theme_Support::TRANSITIONAL_MODE_SLUG; + } elseif ( 'disabled' === $options[ Option::THEME_SUPPORT ] ) { /* * Prior to 1.2, the theme support slug for Reader mode was 'disabled'. This would be saved in options for * themes that had 'amp' theme support defined. Also prior to 1.2, the user could not switch between modes @@ -106,7 +108,7 @@ public static function get_options() { * become 'transitional'. Otherwise, if the theme lacks 'amp' theme support, then this will become the * default 'reader' mode. */ - $options['theme_support'] = $defaults['theme_support']; + $options[ Option::THEME_SUPPORT ] = $defaults[ Option::THEME_SUPPORT ]; } unset( @@ -115,28 +117,30 @@ public static function get_options() { * * @since 1.4.0 */ - $options['auto_accept_sanitization'], + $options[ Option::AUTO_ACCEPT_SANITIZATION ], /** * Remove Story related options. * + * Option::ENABLE_AMP_STORIES was added in 1.2-beta and later migrated into the `experiences` option. + * * @since 1.5.0 */ - $options['story_templates_version'], - $options['story_export_base_url'], - $options['story_settings'], - $options['enable_amp_stories'], // This was added in 1.2-beta and later migrated into the `experiences` option. + $options[ Option::STORY_TEMPLATES_VERSION ], + $options[ Option::STORY_EXPORT_BASE_URL ], + $options[ Option::STORY_SETTINGS ], + $options[ Option::ENABLE_AMP_STORIES ], /** * Remove 'experiences' option. * * @since 1.5.0 */ - $options['experiences'], + $options[ Option::EXPERIENCES ], /** * Remove 'enable_response_caching' option. * * @since 1.5.0 */ - $options['enable_response_caching'] + $options[ Option::ENABLE_RESPONSE_CACHING ] ); return $options; @@ -179,24 +183,24 @@ public static function validate_options( $new_options ) { AMP_Theme_Support::TRANSITIONAL_MODE_SLUG, AMP_Theme_Support::STANDARD_MODE_SLUG, ]; - if ( isset( $new_options['theme_support'] ) && in_array( $new_options['theme_support'], $recognized_theme_supports, true ) ) { - $options['theme_support'] = $new_options['theme_support']; + if ( isset( $new_options[ Option::THEME_SUPPORT ] ) && in_array( $new_options[ Option::THEME_SUPPORT ], $recognized_theme_supports, true ) ) { + $options[ Option::THEME_SUPPORT ] = $new_options[ Option::THEME_SUPPORT ]; // If this option was changed, display a notice with the new template mode. - if ( self::get_option( 'theme_support' ) !== $new_options['theme_support'] ) { + if ( self::get_option( Option::THEME_SUPPORT ) !== $new_options[ Option::THEME_SUPPORT ] ) { add_action( 'update_option_' . self::OPTION_NAME, [ __CLASS__, 'handle_updated_theme_support_option' ] ); } } // Validate post type support. - if ( isset( $new_options['supported_post_types'] ) ) { - $options['supported_post_types'] = []; + if ( isset( $new_options[ Option::SUPPORTED_POST_TYPES ] ) ) { + $options[ Option::SUPPORTED_POST_TYPES ] = []; - foreach ( $new_options['supported_post_types'] as $post_type ) { + foreach ( $new_options[ Option::SUPPORTED_POST_TYPES ] as $post_type ) { if ( ! post_type_exists( $post_type ) ) { add_settings_error( self::OPTION_NAME, 'unknown_post_type', __( 'Unrecognized post type.', 'amp' ) ); } else { - $options['supported_post_types'][] = $post_type; + $options[ Option::SUPPORTED_POST_TYPES ][] = $post_type; } } } @@ -205,21 +209,21 @@ public static function validate_options( $new_options ) { $is_template_support_required = ( isset( $theme_support_args['templates_supported'] ) && 'all' === $theme_support_args['templates_supported'] ); if ( ! $is_template_support_required && ! isset( $theme_support_args['available_callback'] ) ) { - $options['all_templates_supported'] = ! empty( $new_options['all_templates_supported'] ); + $options[ Option::ALL_TEMPLATES_SUPPORTED ] = ! empty( $new_options[ Option::ALL_TEMPLATES_SUPPORTED ] ); // Validate supported templates. - $options['supported_templates'] = []; - if ( isset( $new_options['supported_templates'] ) ) { - $options['supported_templates'] = array_intersect( - $new_options['supported_templates'], + $options[ Option::SUPPORTED_TEMPLATES ] = []; + if ( isset( $new_options[ Option::SUPPORTED_TEMPLATES ] ) ) { + $options[ Option::SUPPORTED_TEMPLATES ] = array_intersect( + $new_options[ Option::SUPPORTED_TEMPLATES ], array_keys( AMP_Theme_Support::get_supportable_templates() ) ); } } // Validate analytics. - if ( isset( $new_options['analytics'] ) ) { - foreach ( $new_options['analytics'] as $id => $data ) { + if ( isset( $new_options[ Option::ANALYTICS ] ) ) { + foreach ( $new_options[ Option::ANALYTICS ] as $id => $data ) { // Check save/delete pre-conditions and proceed if correct. if ( empty( $data['type'] ) || empty( $data['config'] ) ) { @@ -245,16 +249,16 @@ public static function validate_options( $new_options ) { $entry_id = substr( md5( $entry_vendor_type . $entry_config ), 0, 12 ); // Avoid duplicates. - if ( isset( $options['analytics'][ $entry_id ] ) ) { + if ( isset( $options[ Option::ANALYTICS ][ $entry_id ] ) ) { add_settings_error( self::OPTION_NAME, 'duplicate_analytics_entry', __( 'Duplicate analytics entry found.', 'amp' ) ); continue; } } if ( isset( $data['delete'] ) ) { - unset( $options['analytics'][ $entry_id ] ); + unset( $options[ Option::ANALYTICS ][ $entry_id ] ); } else { - $options['analytics'][ $entry_id ] = [ + $options[ Option::ANALYTICS ][ $entry_id ] = [ 'type' => $entry_vendor_type, 'config' => $entry_config, ]; @@ -262,8 +266,14 @@ public static function validate_options( $new_options ) { } } + if ( array_key_exists( Option::DISABLE_CSS_TRANSIENT_CACHING, $new_options ) && true === $new_options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] ) { + $options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] = true; + } else { + unset( $options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] ); + } + // Store the current version with the options so we know the format. - $options['version'] = AMP__VERSION; + $options[ Option::VERSION ] = AMP__VERSION; return $options; } @@ -276,11 +286,11 @@ public static function validate_options( $new_options ) { */ public static function check_supported_post_type_update_errors() { // If all templates are supported then skip check since all post types are also supported. This option only applies with standard/transitional theme support. - if ( self::get_option( 'all_templates_supported', false ) && AMP_Theme_Support::READER_MODE_SLUG !== self::get_option( 'theme_support' ) ) { + if ( self::get_option( Option::ALL_TEMPLATES_SUPPORTED, false ) && AMP_Theme_Support::READER_MODE_SLUG !== self::get_option( Option::THEME_SUPPORT ) ) { return; } - $supported_types = self::get_option( 'supported_post_types', [] ); + $supported_types = self::get_option( Option::SUPPORTED_POST_TYPES, [] ); foreach ( AMP_Post_Type_Support::get_eligible_post_types() as $name ) { $post_type = get_post_type_object( $name ); if ( empty( $post_type ) ) { @@ -346,8 +356,8 @@ public static function handle_analytics_submit() { // Ensure request is coming from analytics option form. check_admin_referer( 'analytics-options', 'analytics-options' ); - if ( isset( $_POST['amp-options']['analytics'] ) ) { - self::update_option( 'analytics', wp_unslash( $_POST['amp-options']['analytics'] ) ); + if ( isset( $_POST[ self::OPTION_NAME ][ Option::ANALYTICS ] ) ) { + self::update_option( Option::ANALYTICS, wp_unslash( $_POST[ self::OPTION_NAME ][ Option::ANALYTICS ] ) ); $errors = get_settings_errors( self::OPTION_NAME ); if ( empty( $errors ) ) { @@ -375,7 +385,7 @@ public static function handle_analytics_submit() { */ public static function update_analytics_options( $data ) { _deprecated_function( __METHOD__, '0.6', __CLASS__ . '::update_option' ); - return self::update_option( 'analytics', wp_unslash( $data ) ); + return self::update_option( Option::ANALYTICS, wp_unslash( $data ) ); } /** @@ -529,7 +539,7 @@ public static function insecure_connection_notice() { * Adds a message for an update of the theme support setting. */ public static function handle_updated_theme_support_option() { - $template_mode = self::get_option( 'theme_support' ); + $template_mode = self::get_option( Option::THEME_SUPPORT ); // Make sure post type support has been added for sake of amp_admin_get_preview_permalink(). foreach ( AMP_Post_Type_Support::get_eligible_post_types() as $post_type ) { diff --git a/includes/options/class-amp-options-menu.php b/includes/options/class-amp-options-menu.php index da9638354ae..b313a63b0cc 100644 --- a/includes/options/class-amp-options-menu.php +++ b/includes/options/class-amp-options-menu.php @@ -5,6 +5,8 @@ * @package AMP */ +use AmpProject\AmpWP\Option; + /** * AMP_Options_Menu class. */ @@ -80,7 +82,7 @@ public function add_menu_items() { ); add_settings_field( - 'theme_support', + Option::THEME_SUPPORT, __( 'Template Mode', 'amp' ), [ $this, 'render_theme_support' ], AMP_Options_Manager::OPTION_NAME, @@ -91,7 +93,7 @@ public function add_menu_items() { ); add_settings_field( - 'supported_templates', + Option::SUPPORTED_TEMPLATES, __( 'Supported Templates', 'amp' ), [ $this, 'render_supported_templates' ], AMP_Options_Manager::OPTION_NAME, @@ -246,7 +248,7 @@ public function render_supported_templates() {

@@ -272,7 +274,7 @@ public function render_supported_templates() {

diff --git a/includes/options/views/class-amp-analytics-options-submenu-page.php b/includes/options/views/class-amp-analytics-options-submenu-page.php index 17eb250eb27..2d5f9521b45 100644 --- a/includes/options/views/class-amp-analytics-options-submenu-page.php +++ b/includes/options/views/class-amp-analytics-options-submenu-page.php @@ -5,6 +5,8 @@ * @package AMP */ +use AmpProject\AmpWP\Option; + /** * Class AMP_Analytics_Options_Submenu_Page */ @@ -186,7 +188,7 @@ protected function render_scripts() { public function render() { $this->render_styles(); - $analytics_entries = AMP_Options_Manager::get_option( 'analytics', [] ); + $analytics_entries = AMP_Options_Manager::get_option( Option::ANALYTICS, [] ); $this->render_title( ! empty( $analytics_entries ) ); diff --git a/includes/sanitizers/class-amp-accessibility-sanitizer.php b/includes/sanitizers/class-amp-accessibility-sanitizer.php new file mode 100644 index 00000000000..643b93b26dd --- /dev/null +++ b/includes/sanitizers/class-amp-accessibility-sanitizer.php @@ -0,0 +1,68 @@ +add_role_and_tabindex_to_on_tap_actors(); + } + + /** + * Adds the role and tabindex attributes to all elements that use a tap action via AMP's "on" event. + */ + public function add_role_and_tabindex_to_on_tap_actors() { + $predicates = [ + '@on', + 'contains( @on, "tap:" )', + 'not( @tabindex ) or not( @role )', + 'not( self::button )', + 'not( self::a[ @href ] )', + ]; + + $expression = sprintf( + '//*[ %s ]', + implode( + ' and ', + array_map( + static function ( $predicate ) { + return "( $predicate )"; + }, + $predicates + ) + ) + ); + + $attributes = [ + Attribute::ROLE => Role::BUTTON, + Attribute::TABINDEX => '0', + ]; + + /** + * Element. + * + * @var DOMElement $element + */ + foreach ( $this->dom->xpath->query( $expression ) as $element ) { + foreach ( $attributes as $attribute_name => $attribute_value ) { + if ( ! $element->hasAttribute( $attribute_name ) ) { + $element->setAttribute( $attribute_name, $attribute_value ); + } + } + } + } +} diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index ebae37373bf..0cc32de0ec0 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -6,7 +6,9 @@ * @since 1.0 */ +use AmpProject\Attribute; use AmpProject\Dom\Document; +use AmpProject\Role; /** * Class AMP_Core_Theme_Sanitizer @@ -54,15 +56,15 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { * @var array */ protected static $modal_roles = [ - 'navigation', - 'menu', - 'search', - 'alert', - 'figure', - 'form', - 'img', - 'toolbar', - 'tooltip', + Role::NAVIGATION, + Role::MENU, + Role::SEARCH, + Role::ALERT, + Role::FIGURE, + Role::FORM, + Role::IMG, + Role::TOOLBAR, + Role::TOOLTIP, ]; /** @@ -602,13 +604,13 @@ public static function remove_actions( $actions = [] ) { public function add_smooth_scrolling( $link_xpaths ) { foreach ( $link_xpaths as $link_xpath ) { foreach ( $this->dom->xpath->query( $link_xpath ) as $link ) { - if ( $link instanceof DOMElement && preg_match( '/#(.+)/', $link->getAttribute( 'href' ), $matches ) ) { - $link->setAttribute( 'on', sprintf( 'tap:%s.scrollTo(duration=600)', $matches[1] ) ); + if ( $link instanceof DOMElement && preg_match( '/#(.+)/', $link->getAttribute( Attribute::HREF ), $matches ) ) { + $link->setAttribute( Attribute::ON, sprintf( 'tap:%s.scrollTo(duration=600)', $matches[1] ) ); // Prevent browser from jumping immediately to the link target. - $link->removeAttribute( 'href' ); - $link->setAttribute( 'tabindex', '0' ); - $link->setAttribute( 'role', 'button' ); + $link->removeAttribute( Attribute::HREF ); + $link->setAttribute( Attribute::TABINDEX, '0' ); + $link->setAttribute( Attribute::ROLE, Role::BUTTON ); } } } @@ -1026,7 +1028,7 @@ public function add_twentyseventeen_sticky_nav_menu() { * * @var DOMElement $link */ - $link->setAttribute( 'tabindex', '-1' ); + $link->setAttribute( Attribute::TABINDEX, '-1' ); } $navigation_top->parentNode->insertBefore( $navigation_top_fixed, $navigation_top->nextSibling ); @@ -1551,8 +1553,8 @@ public function add_twentyfourteen_slider_carousel() { $a->setAttribute( 'class', 'slider-active' ); } $a->setAttribute( Document::AMP_BIND_DATA_ATTR_PREFIX . 'class', "$selected_slide_state_id == $i ? 'slider-active' : ''" ); - $a->setAttribute( 'role', 'button' ); - $a->setAttribute( 'on', "tap:AMP.setState( { $selected_slide_state_id: $i } )" ); + $a->setAttribute( Attribute::ROLE, Role::BUTTON ); + $a->setAttribute( Attribute::ON, "tap:AMP.setState( { $selected_slide_state_id: $i } )" ); $li->setAttribute( 'option', (string) $i ); $a->appendChild( $this->dom->createTextNode( $i + 1 ) ); $li->appendChild( $a ); @@ -1599,9 +1601,9 @@ public function add_twentyfourteen_search() { $search_input_el->setAttribute( 'id', $search_input_id ); $on .= ",$search_input_id.focus()"; } - $search_toggle_link->setAttribute( 'on', $on ); - $search_toggle_link->setAttribute( 'tabindex', '0' ); - $search_toggle_link->setAttribute( 'role', 'button' ); + $search_toggle_link->setAttribute( Attribute::ON, $on ); + $search_toggle_link->setAttribute( Attribute::TABINDEX, '0' ); + $search_toggle_link->setAttribute( Attribute::ROLE, Role::BUTTON ); // Set visibility and aria-expanded based of the link based on whether the search bar is expanded. $search_toggle_link->setAttribute( 'aria-expanded', wp_json_encode( $hidden ) ); @@ -1688,9 +1690,9 @@ public function wrap_modal_in_lightbox( $args = [] ) { * * @var DOMElement $event_element */ - $event_element->setAttribute( 'role', $this->guess_modal_role( $modal_content_node ) ); + $event_element->setAttribute( Attribute::ROLE, $this->guess_modal_role( $modal_content_node ) ); // Setting tabindex to -1 (not reachable) as keyboard focus is handled through toggles. - $event_element->setAttribute( 'tabindex', -1 ); + $event_element->setAttribute( Attribute::TABINDEX, -1 ); } $parent_node = $modal_content_node->parentNode; @@ -2012,11 +2014,11 @@ protected function xpath_from_css_selector( $css_selector ) { */ protected function guess_modal_role( DOMElement $modal ) { // No classes to base our guess on, so keep it generic. - if ( ! $modal->hasAttribute( 'class' ) ) { - return 'dialog'; + if ( ! $modal->hasAttribute( Attribute::CLASS_ ) ) { + return Role::DIALOG; } - $classes = preg_split( '/\s+/', trim( $modal->getAttribute( 'class' ) ) ); + $classes = preg_split( '/\s+/', trim( $modal->getAttribute( Attribute::CLASS_ ) ) ); foreach ( self::$modal_roles as $role ) { if ( in_array( $role, $classes, true ) ) { @@ -2025,6 +2027,6 @@ protected function guess_modal_role( DOMElement $modal ) { } // None of the roles we are looking for match any of the classes. - return 'dialog'; + return Role::DIALOG; } } diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index 3542fa5e86b..03b5a183965 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -7,6 +7,7 @@ */ use AmpProject\DevMode; +use AmpProject\Dom\Document; /** * Class AMP_Form_Sanitizer @@ -201,17 +202,14 @@ public function ensure_response_message_elements( $form ) { 'submitting' => null, ]; - $templates = $form->getElementsByTagName( 'template' ); - for ( $i = $templates->length - 1; $i >= 0; $i-- ) { - /** - * Parent node. - * - * @var DOMElement $parent - */ - $parent = $templates->item( $i )->parentNode; - foreach ( array_keys( $elements ) as $attribute ) { - if ( $parent->hasAttribute( $attribute ) ) { - $elements[ $attribute ] = $parent; + $templates = $this->dom->xpath->query( Document::XPATH_MUSTACHE_TEMPLATE_ELEMENTS_QUERY, $form ); + foreach ( $templates as $template ) { + $parent = $template->parentNode; + if ( $parent instanceof DOMElement ) { + foreach ( array_keys( $elements ) as $attribute ) { + if ( $parent->hasAttribute( $attribute ) ) { + $elements[ $attribute ] = $parent; + } } } } diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 9f96a9b89c9..62814c5387a 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -5,7 +5,6 @@ * @package AMP */ -use AmpProject\AmpWP\BackgroundTask\MonitorCssTransientCaching; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\RemoteRequest\CachedRemoteGetRequest; use AmpProject\AmpWP\RemoteRequest\WpHttpRemoteGetRequest; @@ -2668,7 +2667,7 @@ private function collect_inline_styles( DOMElement $element ) { // Skip processing stylesheets that contain mustache template variables if the element is inside of a mustache template. if ( preg_match( '/{{[^}]+?}}/', $value ) && - 0 !== $this->dom->xpath->query( '//template[ @type="amp-mustache" ]//.', $element )->length + 0 !== $this->dom->xpath->query( '//template[ @type="amp-mustache" ]//.|//script[ @template="amp-mustache" and @type="text/plain" ]//.', $element )->length ) { return; } diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php index 15b5d516e10..6c9cd9e504b 100644 --- a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php +++ b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php @@ -5,8 +5,11 @@ * @package AMP */ +use AmpProject\Attribute; use AmpProject\CssLength; use AmpProject\Dom\Document; +use AmpProject\Extension; +use AmpProject\Tag; /** * Strips the tags and attributes from the content that are not allowed by the AMP spec. @@ -1255,7 +1258,7 @@ private function sanitize_disallowed_attribute_values_in_node( DOMElement $node, // Check the context to see if we are currently within a template tag. // If this is the case and the attribute value contains a template placeholder, we skip sanitization. - if ( ! empty( $this->open_elements['template'] ) && preg_match( '/{{[^}]+?}}/', $attr_node->nodeValue ) ) { + if ( $this->is_inside_mustache_template( $node ) && preg_match( '/{{[^}]+?}}/', $attr_node->nodeValue ) ) { continue; } @@ -1340,6 +1343,10 @@ private function is_valid_layout( $tag_spec, $node ) { return true; } + if ( $this->is_inside_mustache_template( $node ) && $this->has_layout_attribute_with_mustache_variable( $node ) ) { + return true; + } + $layout_attr = $node->getAttribute( 'layout' ); $allow_fluid = AMP_Rule_Spec::LAYOUT_FLUID === $layout_attr; $allow_auto = true; @@ -1447,6 +1454,56 @@ private function is_valid_layout( $tag_spec, $node ) { return true; } + /** + * Whether the node is inside a mustache template. + * + * @since 1.5.3 + * + * @param DOMElement $node The node to examine. + * @return bool Whether the node is inside a valid mustache template. + */ + private function is_inside_mustache_template( DOMElement $node ) { + if ( ! empty( $this->open_elements[ Tag::TEMPLATE ] ) ) { + while ( $node->parentNode instanceof DOMElement ) { + $node = $node->parentNode; + if ( Tag::TEMPLATE === $node->nodeName && Extension::MUSTACHE === $node->getAttribute( Attribute::TYPE ) ) { + return true; + } + } + } elseif ( ! empty( $this->open_elements[ Tag::SCRIPT ] ) ) { + while ( $node->parentNode instanceof DOMElement ) { + $node = $node->parentNode; + if ( Tag::SCRIPT === $node->nodeName && Extension::MUSTACHE === $node->getAttribute( Attribute::TEMPLATE ) && Attribute::TYPE_TEXT_PLAIN === $node->getAttribute( Attribute::TYPE ) ) { + return true; + } + } + } + return false; + } + + /** + * Whether the node has a layout attribute with variable syntax, like {{foo}}. + * + * This is important for whether to validate the layout of the node. + * Similar to the validation logic in the AMP validator. + * + * @see https://github.com/ampproject/amphtml/blob/c083d2c6120a251dcc9b0beb33c0336c7d3ca5a8/validator/engine/validator.js#L4038-L4054 + * + * @since 1.5.3 + * + * @param DOMElement $node The node to examine. + * @return bool Whether the node has a layout attribute with variable syntax. + */ + private function has_layout_attribute_with_mustache_variable( DOMElement $node ) { + foreach ( [ Attribute::LAYOUT, Attribute::WIDTH, Attribute::HEIGHT, Attribute::SIZES, Attribute::HEIGHTS ] as $attribute ) { + if ( preg_match( '/{{[^}]+?}}/', $node->getAttribute( $attribute ) ) ) { + return true; + } + } + + return false; + } + /** * Calculate the effective width from the input layout and input width. * diff --git a/includes/settings/class-amp-customizer-design-settings.php b/includes/settings/class-amp-customizer-design-settings.php index 154f732fe08..f345d93d159 100644 --- a/includes/settings/class-amp-customizer-design-settings.php +++ b/includes/settings/class-amp-customizer-design-settings.php @@ -5,6 +5,8 @@ * @package AMP */ +use AmpProject\AmpWP\Option; + /** * Class AMP_Customizer_Design_Settings */ @@ -41,7 +43,7 @@ class AMP_Customizer_Design_Settings { */ public static function is_amp_customizer_enabled() { - if ( AMP_Theme_Support::READER_MODE_SLUG !== AMP_Options_Manager::get_option( 'theme_support' ) ) { + if ( AMP_Theme_Support::READER_MODE_SLUG !== AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) ) { return false; } diff --git a/includes/templates/class-amp-post-template.php b/includes/templates/class-amp-post-template.php index 4829c13b1a4..a68dcebce29 100644 --- a/includes/templates/class-amp-post-template.php +++ b/includes/templates/class-amp-post-template.php @@ -342,8 +342,17 @@ private function build_post_comments_data() { * Build post content. */ private function build_post_content() { - /** This filter is documented in wp-includes/post-template.php */ - $content = apply_filters( 'the_content', get_the_content( null, false, $this->post ) ); + if ( post_password_required( $this->post ) ) { + $content = get_the_password_form( $this->post ); + } else { + /** + * This filter is documented in wp-includes/post-template.php. + * + * Note: This is intentionally not using get_the_content() because the legacy behavior of posts in + * Reader mode is to display multi-page posts as a single page without any pagination links. + */ + $content = apply_filters( 'the_content', $this->post->post_content ); + } $this->add_data_by_key( 'post_amp_content', $content ); } diff --git a/lib/common/src/Attribute.php b/lib/common/src/Attribute.php index 8321e408596..b51812fbbc7 100644 --- a/lib/common/src/Attribute.php +++ b/lib/common/src/Attribute.php @@ -46,11 +46,14 @@ interface Attribute const LOOP = 'loop'; const MEDIA = 'media'; const NAME = 'name'; + const ON = 'on'; const PROFILE = 'profile'; const REL = 'rel'; const ROLE = 'role'; const SIZES = 'sizes'; const SRC = 'src'; + const TABINDEX = 'tabindex'; + const TEMPLATE = 'template'; const TYPE = 'type'; const VIEWPORT = 'viewport'; const WIDTH = 'width'; @@ -59,9 +62,12 @@ interface Attribute const ALL_AMP4ADS = [self::AMP4ADS, self::AMP4ADS_EMOJI, self::AMP4ADS_EMOJI_ALT]; const ALL_AMP4EMAIL = [self::AMP4EMAIL, self::AMP4EMAIL_EMOJI, self::AMP4EMAIL_EMOJI_ALT]; - const TYPE_HTML = 'text/html'; - const TYPE_JSON = 'application/json'; - const TYPE_LD_JSON = 'application/ld+json'; + const ALL_BOILERPLATES = [self::AMP_BOILERPLATE, self::AMP4ADS_BOILERPLATE, self::AMP4EMAIL_BOILERPLATE]; + + const TYPE_HTML = 'text/html'; + const TYPE_JSON = 'application/json'; + const TYPE_LD_JSON = 'application/ld+json'; + const TYPE_TEXT_PLAIN = 'text/plain'; const REL_CANONICAL = 'canonical'; const REL_DNS_PREFETCH = 'dns-prefetch'; diff --git a/lib/common/src/Dom/Document.php b/lib/common/src/Dom/Document.php index c41364d451d..3e0a64a2526 100644 --- a/lib/common/src/Dom/Document.php +++ b/lib/common/src/Dom/Document.php @@ -119,6 +119,13 @@ final class Document extends DOMDocument */ const XPATH_URL_ENCODED_ATTRIBUTES_QUERY = './/*/@src|.//*/@href|.//*/@action'; + /** + * Xpath query to fetch the elements containing Mustache templates (both