Skip to content

desktop_mode_is_enabled() should honor the desktop_mode_mode_enabled filter #105

@epeicher

Description

@epeicher

Summary

The desktop_mode_mode_enabled filter is the documented surface for gating which users get Desktop Mode. Today it is enforced at three call sites (the AJAX save endpoint, the /desktop-mode/ portal, and now the admin-bar toggle from #98) but NOT in the central desktop_mode_is_enabled() helper, which only reads user meta.

This means a user who set the user meta in the past and then had the filter flipped against them will still register as "enabled" everywhere desktop_mode_is_enabled() is consulted as a render-time gate. The filter is leaky.

Evidence

includes/helpers.php:17-23 (current implementation):

function desktop_mode_is_enabled( $user_id = 0 ) {
    if ( ! $user_id ) {
        $user_id = get_current_user_id();
    }
    return '1' === (string) get_user_meta( $user_id, 'desktop_mode_mode', true );
}

The filter is never consulted. The doc example at docs/examples/gate-by-role.md previously claimed it WAS, that doc was corrected to match code in #98, but the original wording is probably the intended contract.

Filter enforcement today (only here):

Proposed change

Make desktop_mode_is_enabled() AND on the meta AND on apply_filters( 'desktop_mode_mode_enabled', true, $user_id ). After the change:

function desktop_mode_is_enabled( $user_id = 0 ) {
    if ( ! $user_id ) {
        $user_id = get_current_user_id();
    }
    if ( '1' !== (string) get_user_meta( $user_id, 'desktop_mode_mode', true ) ) {
        return false;
    }
    return (bool) apply_filters( 'desktop_mode_mode_enabled', true, $user_id );
}

Risk and scope

This is a behavior change at every call site of desktop_mode_is_enabled(), used as a render-time gate in many places (chromeless rendering, admin-bar, payload generation, REST permission checks, etc.). Most call sites should benefit from the tighter gate. Risks worth thinking through before shipping:

  • A user with the meta set whose filter newly returns false will be kicked back to classic admin on next page load. That is the intended behavior of the filter, but worth documenting prominently.
  • Tests that bypass the filter (set the meta directly, then assert enabled) will need updating to also stub the filter or accept the new contract.

Suggested fix

  1. Update desktop_mode_is_enabled() per above.
  2. Add a unit test confirming the filter overrides positive meta.
  3. Audit existing callers for any that might be surprised; document the change in docs/hooks-reference.md and docs/examples/gate-by-role.md.
  4. CHANGELOG entry, this is a contract-tightening, technically a behavior change.

Related

Follow-up to #98.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions