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

AMP: Jetpack now always hides the admin bar #15353

Closed
jeherve opened this issue Apr 7, 2020 · 5 comments · Fixed by #15372
Closed

AMP: Jetpack now always hides the admin bar #15353

jeherve opened this issue Apr 7, 2020 · 5 comments · Fixed by #15372
Assignees
Labels
AMP [Pri] High [Type] Bug When a feature is broken and / or not performing as intended
Milestone

Comments

@jeherve
Copy link
Member

jeherve commented Apr 7, 2020

This was introduced in #14840.

I do not think this is right. It is causing the admin bar to be hidden on all AMP pages, even when it should be displayed.

In AMP plugin 1.3 we introduced AMP dev mode support to exclude the admin bar from being subject to AMP validation constraints, allowing administrative scripts and styles to work as expected even on AMP pages.

More info: weston.ruter.net/2019/09/24/integrating-with-amp-dev-mode-in-wordpress

-- #14840 (comment)

if the issue is the scripts and styles that the Jetpack Likes module includes, a better way to resolve this I think is to explicitly mark them for AMP dev mode by adding admin-bar as a dependency for the scripts and styles.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Pri] High AMP labels Apr 7, 2020
@jeherve jeherve added this to the 8.5 milestone Apr 7, 2020
@westonruter
Copy link
Contributor

westonruter commented Apr 7, 2020

A quick workaround to restore the admin bar in AMP pages is via this plugin code:

add_filter( 'show_admin_bar', 'is_user_logged_in', 11 );

In plugin form: https://gist.github.com/westonruter/d168c290d6c01c107da960d48fa3dad3

But I'm not clear specifically why the admin bar was removed in the first place, other than it being an easy way to prevent AMP-invalid scripts/styles from being added to the page.

@kitchin
Copy link
Contributor

kitchin commented Apr 8, 2020

Probably the same bug: https://wordpress.org/support/topic/amp-1-5-2-completely-breaks-site/

Jetpack and Amp have this weird interaction, but a third plugin is calling is_admin_bar_showing() which causes it to happen. That call is on the action plugins_loaded so I don't think you can say the third plugin is doing it too early.

See the traceback in the support topic above: https://wordpress.org/support/topic/amp-1-5-2-completely-breaks-site/page/2/#post-12643000

@jeherve jeherve modified the milestones: 8.5, 8.4.2 Apr 8, 2020
@jeherve jeherve self-assigned this Apr 8, 2020
jeherve added a commit that referenced this issue Apr 8, 2020
Fixes #15353

The admin bar can be used in AMP views since AMP 1.3.1, thanks to AMP Dev mode.
@philipjohn
Copy link
Contributor

FYI we've already had a publisher reach out to both Happiness and Newspack team about this as they were confused where the post edit link had gone when viewing articles.

@westonruter
Copy link
Contributor

Another report on the AMP plugin forum: https://wordpress.org/support/topic/amp-endpoint-error/

@westonruter
Copy link
Contributor

A quick workaround to restore the admin bar in AMP pages is via this plugin code:

add_filter( 'show_admin_bar', 'is_user_logged_in', 11 );

In plugin form: https://gist.github.com/westonruter/d168c290d6c01c107da960d48fa3dad3

Here's a better workaround that also prevents the is_amp_endpoint() notices from occurring when is_admin_bar_showing() is called before the wp action:

// Unhook the show_admin_bar filter added in Jetpack_AMP_Support::init().
add_action(
	'init',
	function () {
		remove_filter( 'show_admin_bar', [ 'Jetpack_AMP_Support', 'show_admin_bar' ] );
	},
	2 // Because Jetpack_AMP_Support::init() happens at priority 1.
);

jeherve added a commit that referenced this issue Apr 14, 2020
* AMP: allow the display of the admin bar in AMP views.

Fixes #15353

The admin bar can be used in AMP views since AMP 1.3.1, thanks to AMP Dev mode.

* Likes: do not load Likes in admin bar on AMP views
jeherve added a commit that referenced this issue Apr 14, 2020
…15372)

* AMP: allow the display of the admin bar in AMP views.

Fixes #15353

The admin bar can be used in AMP views since AMP 1.3.1, thanks to AMP Dev mode.

* Likes: do not load Likes in admin bar on AMP views
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMP [Pri] High [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants