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 error if admin bar styles are removed #7766

Open
jcvignoli opened this issue Mar 25, 2024 · 3 comments
Open

AMP error if admin bar styles are removed #7766

jcvignoli opened this issue Mar 25, 2024 · 3 comments
Labels
Bug Something isn't working P2 Low priority
Projects
Milestone

Comments

@jcvignoli
Copy link

Bug Description

If visiting a wordpress page where I removed admin bar styles, AMP throws
Warning: Undefined array key "admin-bar" in XXX/wp-content/plugins/amp/includes/class-amp-theme-support.php on line 1382
I removed the styles (and the whole admin bar) with

add_filter( 'show_admin_bar', '__return_false' );
wp_deregister_style( 'admin-bar' );

If I take out wp_deregister_style( 'admin-bar' ); it works again.

Expected Behaviour

Should not throw warning

Screenshots

No response

PHP Version

8.2

Plugin Version

2.5.3

AMP plugin template mode

Standard

WordPress Version

No response

Site Health

No response

Gutenberg Version

No response

OS(s) Affected

No response

Browser(s) Affected

No response

Device(s) Affected

No response

Acceptance Criteria

No response

Implementation Brief

on line 1382 includes/class-amp-theme-support.php you should replace
is_array( wp_styles()->registered['admin-bar']->deps ) && in_array( $handle, wp_styles()->registered['admin-bar']->deps, true ) ? [..]
by
isset( wp_styles()->registered['admin-bar']->deps ) && is_array( wp_styles()->registered['admin-bar']->deps ) && in_array( $handle, wp_styles()->registered['admin-bar']->deps, true ) ? [..]
(add an isset())

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

@jcvignoli jcvignoli added the Bug Something isn't working label Mar 25, 2024
@westonruter
Copy link
Member

I don't think you should every fully deregister the style. Shouldn't you rather dequeue it? Use wp_dequeue_style( 'admin-bar' ) instead.

@jcvignoli
Copy link
Author

Thanks for the suggestion, I'll look into that. But adding an isset() is a good practice anyway, one should always check if a property exists before using it, don't you think so?

@westonruter
Copy link
Member

Yes, that would be good defensive coding.

@westonruter westonruter added this to the v2.5.4 milestone May 2, 2024
@westonruter westonruter added this to Backlog in Ongoing via automation May 2, 2024
@westonruter westonruter moved this from Backlog to To Do in Ongoing May 2, 2024
@westonruter westonruter added the P2 Low priority label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working P2 Low priority
Projects
Ongoing
  
To Do
Development

No branches or pull requests

2 participants