WordPress.org

Make WordPress Core

Opened 9 months ago

Last modified 5 months ago

#45529 accepted defect (bug)

Correct phpdoc example for Internal Pointers

Reported by: daleharrison Owned by: garrett-eclipse
Milestone: 5.3 Priority: normal
Severity: normal Version: 3.3
Component: Administration Keywords: has-patch needs-refresh 2nd-opinion
Focuses: docs Cc:

Description

I'm trying to disable the "Personal Data and Privacy" pointer for all users.

Unfortunately, it does not seem to be possible to do so via the remove_action() approach noted at the top of the WP_Internal_Pointers class.

This is the suggested approach:

<?php
remove_action( 'admin_print_footer_scripts', array( 'WP_Internal_Pointers', 'pointer_wp390_widgets' ) );

…but I'm not getting the desired result when applied like so:

<?php
remove_action( 'admin_print_footer_scripts', array( 'WP_Internal_Pointers', 'wp496_privacy' ) );

It is also worth noting that the developer of a plugin called Dismiss Privacy Nag seems to be bumping into the same issue: https://wordpress.org/plugins/dismiss-privacy-nag/#description.

Attachments (1)

45529.diff (916 bytes) - added by garrett-eclipse 7 months ago.

Download all attachments as: .zip

Change History (21)

#1 follow-up: @swissspidy
9 months ago

  • Keywords close added

You should be able to do this:

<?php
add_action(
  'admin_enqueue_scripts',
  function() {
    remove_action( 'admin_print_footer_scripts', array( 'WP_Internal_Pointers', 'wp496_privacy' ) );
  }
};

#2 in reply to: ↑ 1 @daleharrison
8 months ago

Replying to swissspidy:

You should be able to do this:

<?php
add_action(
  'admin_enqueue_scripts',
  function() {
    remove_action( 'admin_print_footer_scripts', array( 'WP_Internal_Pointers', 'wp496_privacy' ) );
  }
);

Thanks, but unfortunately that's not working either. I tried it in a regular plugin, an mu-plugin, and my theme's functions.php file.

Note: I fixed the closing bracket of the add_action() call as quoted above.

Anything else I'm missing here or is this a legitimate bug?

#3 @SergeyBiryukov
8 months ago

  • Component changed from Administration to Privacy
  • Focuses administration added

#4 @garrett-eclipse
7 months ago

  • Focuses docs added; administration removed
  • Keywords has-patch dev-feedback added; close removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to garrett-eclipse
  • Status changed from new to assigned

Hi @daleharrison

Looking into this I found two changes the snippet you and @swissspidy provided;

  1. Missing pointer_ prefix on pointer_wp496_privacy, all WP_Internal_Pointers are called with a pointer_ prefix on their add_action call.

Code Reference - https://github.com/WordPress/WordPress/blob/56c162fbc9867f923862f64f1b4570d885f1ff03/wp-admin/includes/class-wp-internal-pointers.php#L89

  1. Seems the priority on the admin_enqueue_scripts add_action needs to be 11. I've found for remove_action's it's always best to increment your priority by 1.

Working code sample;

<?php
add_action(
  'admin_enqueue_scripts',
  function() {
    remove_action( 'admin_print_footer_scripts', array( 'WP_Internal_Pointers', 'pointer_wp496_privacy' ) );
  },
  11
);

*This works from functions.php in your theme, tested on twentynineteen.

As well I passed this information to Luciano the dev of the plugin you mentioned as a PR with a working version of the above code.
PR - https://github.com/luciano-croce/dismiss-privacy-nag/pull/1

@swissspidy I added 45529.diff to update the inline phpdoc as the suggested code doesn't function as is, would you mind taking a look.

Thank you

Last edited 7 months ago by garrett-eclipse (previous) (diff)

#5 @Luciano Croce
7 months ago

  • Keywords needs-screenshots added
  • Resolution set to worksforme
  • Status changed from assigned to closed

@garrett-eclipse @swissspidy @daleharrison very good patch.

Tested locally (on my environment) work fine. :)

Work in functions.php of Themes:

  • Twentynineteen
  • Twentyseveteen
  • Twentyeleven

Thanks!

P.S. I have updated both the code of my plugins Dismiss Privacy Nag and Dismiss Privacy Tools now... ;)

#6 @Luciano Croce
7 months ago

  • Keywords needs-screenshots removed

#7 @swissspidy
7 months ago

  • Milestone Future Release deleted

#8 follow-up: @garrett-eclipse
7 months ago

  • Milestone set to Future Release
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Thanks @Luciano Croce

I'm glad that worked out for your plugins.

I'm re-opening this ticket as I added the 45529.diff patch to correct the insufficient phpdoc example as the existing example doesn't account for the need to have the remove_action called from a admin_enqueue_scripts action with priority greater than 10.

@swissspidy would you mind looking at the updated example, I'm unsure if that's the best way to provide a multi-line code sample.

Appreciated

#9 follow-up: @daleharrison
7 months ago

Thanks @garrett-eclipse and @Luciano Croce, nice work. The code above is working perfectly for me in an mu-plugin, which is where I needed it to go.

#10 @garrett-eclipse
7 months ago

  • Status changed from reopened to accepted

Awesome @daleharrison that's great news, thanks for the confirmation it wraps things up with a nice little bow ;)

#11 @desrosj
7 months ago

  • Version changed from 4.9.8 to 4.9.6

I’d like to look into historically how long these pointer notices have remained in core, but I think that we can start exploring potentially removing this notice in either 5.1 or 5.2. It is meant as an upgrade notice to inform users of a new feature.

#12 @garrett-eclipse
7 months ago

  • Summary changed from Can't Remove wp496_privacy From WP_Internal_Pointers to Correct phpdoc example for Internal Pointers

Thanks @desrosj, I agree we should be deprecating it in the near future. I've created #45999 for that purpose and to continue the discussion as well as to ping to open for others.

I've updated this ticket as the phpdoc example that proceeds the pointers function doesn't function without calling it appropriately and w/ priority. For this ticket my goal is simply to correct that example.

#13 in reply to: ↑ 9 @Luciano Croce
7 months ago

Replying to daleharrison:

Thanks @garrett-eclipse and @Luciano Croce, nice work. The code above is working perfectly for me in an mu-plugin, which is where I needed it to go.

Remember that my username for "quote me" is luciano-croce @luciano-croce Thanks!

#14 in reply to: ↑ 8 @Luciano Croce
7 months ago

Replying to garrett-eclipse:

Thanks @Luciano Croce

I'm glad that worked out for your plugins.

I'm re-opening this ticket as I added the 45529.diff patch to correct the insufficient phpdoc example as the existing example doesn't account for the need to have the remove_action called from a admin_enqueue_scripts action with priority greater than 10.

@swissspidy would you mind looking at the updated example, I'm unsure if that's the best way to provide a multi-line code sample.

Appreciated

Remember that my username for "quote me" is luciano-croce @luciano-croce Thanks!

#15 @garrett-eclipse
7 months ago

Hi @luciano-croce, yes seems your username on Trac is broken. When selecting you from the auto-suggestion and when adding a reply to you it uses @Luciano Croce which breaks on the space.
See screen here for the autosuggestion issue - https://i.imgur.com/68lr2rw.png

Can you raise in #meta to have your username fixed on Trac as the system is using an invalid one and I doubt anyone will remember.

Thanks

#16 @Luciano Croce
7 months ago

Sorry Brother!

Yes I know but IMHO the problem is (also) a "fake user" called @luciano that do not exist!

#17 @desrosj
6 months ago

  • Component changed from Privacy to Administration
  • Keywords needs-refresh added; dev-feedback removed
  • Milestone changed from Future Release to 5.2

Since this is a documentation improvement, let's fix this in 5.2. Also, pointers are an Administration feature.

The updated example in 45529.diff uses an anonymous function, which is only supported in PHP 5.3+.

Instead of changing the example, can it just note the action hook and priority?

#18 @desrosj
6 months ago

  • Version changed from 4.9.6 to 3.3

Tracked the introduction of this example to [19388].

#19 @garrett-eclipse
6 months ago

Thanks @desrosj 5.2 sounds good.

I'm not sure the best approach there as the example will also become outdated once the pointer in the example is deprecated. Do you know of any other instances where a code sample is provided in the phpdoc block?

Possibly the example should be moved into a codex page or somewhere we can then use a permalink to replace the code example with?

I did look at how we would note the action hook and priority but was hard to come up with something understandable as it's a double function and the remove action uses an array to define the WP_Internal_Pointers class. What I was left with on my attempt to write something was;
"Individual pointers can be disabled by wrapping an add_action for admin_enqueue_scripts with a priority of 11 around a remove_action for admin_print_footer_scripts to remove the pointer function name with point_ prefix from the WP_Internal_Pointers class"
*I tried to rewrite the function using the above and was hard pressed, I feel the example is worth 1000 words in this case as it clearly shows something that can be copy pasted.

Would love to get your thoughts and can refresh accordingly.

#20 @garrett-eclipse
5 months ago

  • Keywords 2nd-opinion added
  • Milestone changed from 5.2 to 5.3

Moving this off of the 5.2 milestone as it's no longer a priority seeing that r44787 is now in trunk which removes the privacy pointer leaving no active pointers.

Added 2nd-opinion to get feedback as to the correct approach to introducing a code block to phpdoc and if it should be a @link insteat.

Note: See TracTickets for help on using tickets.