Extend Sandboxing experiment to Paired AMP modes #7288
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mode - Replace link[rel=amphtml] with link[rel=alternate] if non-strict sandboxing level is selected.
westonruter
reviewed
Oct 20, 2022
… and in reader mode
thelovekesh
force-pushed
the
add/sandboxing-in-all-modes
branch
from
288cbe9 to
40688e0
Compare
thelovekesh
commented
Nov 2, 2022
westonruter
reviewed
Nov 2, 2022
Member
|
For this existing code: amp-wp/src/MobileRedirection.php Lines 573 to 583 in 40688e0 It should probably afterward do: if ( empty( $text ) ) {
return;
}This would allow someone to turn off the link via: add_filter( 'amp_mobile_version_switcher_styles_used', '__return_false' );
add_filter( 'amp_mobile_version_switcher_link_text', '__return_empty_string' ); |
…direction is disabled
Contributor
|
Plugin builds for dbe9350 are ready 🛎️!%0A- Download development build%0A- Download production build |
Co-authored-by: Weston Ruter <[email protected]>
Codecov Report
@@ Coverage Diff @@
## develop #7288 +/- ##
==============================================
+ Coverage 64.88% 77.82% +12.94%
- Complexity 0 6867 +6867
==============================================
Files 67 276 +209
Lines 1159 19842 +18683
==============================================
+ Hits 752 15443 +14691
- Misses 407 4399 +3992
Flags with carried forward coverage won't be shown. Click here to find out more.
|
westonruter
requested changes
Nov 22, 2022
thelovekesh
force-pushed
the
add/sandboxing-in-all-modes
branch
from
8cc7b58 to
5e87dbf
Compare
thelovekesh
commented
Nov 23, 2022
thelovekesh
force-pushed
the
add/sandboxing-in-all-modes
branch
from
8bec181 to
0d2c16f
Compare
…_used() Test if it keeps the required AMP markup if AMP elements are present
thelovekesh
commented
Nov 24, 2022
westonruter
requested changes
Nov 29, 2022
Member
westonruter
left a comment
westonruter
left a comment
There was a problem hiding this comment.
I was doing some thorough testing and I found two additional required changes. Almost there!
Comment on lines
+71
to
+73
| if ( ! amp_is_canonical() && ( $is_mobile_redirect_enabled || ( 1 === $sandboxing_level || 2 === $sandboxing_level ) ) ) { | ||
| add_action( 'wp_head', [ $this, 'add_mobile_alternative_link' ] ); | ||
| } |
Member
There was a problem hiding this comment.
Note to self: previously this was done below.
if ( ! amp_is_request() ) {
add_action( 'wp_head', [ $this, 'add_mobile_alternative_link' ] );
}…te link Co-authored-by: Weston Ruter <[email protected]>
Co-authored-by: Weston Ruter <[email protected]>
westonruter
added
the
Changelogged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7268
Checklist