Opened 21 months ago

Closed 16 months ago

Last modified 16 months ago

#6509 closed defect (bug) (fixed)

Forum profile: "Also viewing" checkbox sometimes gets unchecked

Reported by: zoonini's profile zoonini Owned by: clorith's profile Clorith
Milestone: Priority: normal
Component: Support Forums Keywords:


Several people have noticed that the "Enable the Also Viewing feature" checkbox on their profile sometimes gets randomly unchecked on its own.

Profile URL example: /support/users/USERNAME/edit/

To reactivate the feature, the box needs to be manually re-checked.

Slack discussion:

Attachments (1)

also-viewing.jpg (49.3 KB) - added by zoonini 21 months ago.
Forum profile: Also Viewing checkbox

Download all attachments as: .zip

Change History (8)

21 months ago

Forum profile: Also Viewing checkbox

This ticket was mentioned in Slack in #forums by zoonini. View the logs.

21 months ago

#2 @jeroenrotty
21 months ago

This is actually happening more often lately, I had it on September 29th, and now again on October 7nd.

#3 @dd32
20 months ago

  • Component changed from General to Support Forums

This ticket was mentioned in Slack in #forums by zoonini. View the logs.

16 months ago

#5 @Clorith
16 months ago

  • Owner set to Clorith
  • Resolution set to fixed
  • Status changed from new to closed

In 12412:

Support Forums: Check intent before updating user preferences.

The bbPress profile page may be triggered by various scenarios that do not have the full context of a profile edit included.

By introducing a hidden input field in the form areas which need a "yes or no" type response, indicated by checkboxes, we can ensure that these are not unintentionally cleared or set from profile modifications made without the options even being presented to the user.

Props zoonini.
Fixes #6509.

#6 @Otto42
16 months ago

I highly dislike this fix.

The bbPress profile page may be triggered by various scenarios

Could you clarify these scenarios? When does the profile get edited, but is not getting actually edited?

#7 @Clorith
16 months ago

Initially posted this on Slack to the same remark, but responding here for the sake of history :)

So the reason it's happening is because these profile field updates are hooking into bbp_profile_update, which is just an extension that runs through cores own profile_update action.

The latter of these actions is triggered any time a user is inserted, or updated, and by extension so is the bbPress one then. By having user settings tied to a named checkbox's existence in a request means that the code basically says "if this isn't present, the user decided not to tick the box", which isn't true when the user can be updated from "other" places.

In the most recent case, the Two-Factor plugin, as used under the wporg-two-factor one, will update a user and trigger this cascade of actions, and because we (previously) had no safeguards on these specific form fields, they would be treated as "unticked by the user", and be wiped.

Some alternative approaches though, if you'd rather, could for example be;

  • add a nonce for each field, but don't throw a nonce-error if it doesn't pass, just return early.
  • Convert checkboxes to select boxes with yes/no type options and a switch statement in the form handler
  • Specifically check the referrer URL of a post request

I chose not to go the nonce route, since that would add 2 DB entries per user for nonce-references, which seemed overkill.
Likewise, the idea of select boxes for on/off statements is a bit backwards, and referrals are unreliable.

So with that in mind, the implementation of a can_* field entry, that gets posted alongside the value we check for, was the simplest one to both ensure these non-bbPress-core user settings we've implemented do not get wiped from unintentional "external" actions, in a period where users suddenly got a whiff of a new security feature that's been long requested and are likely to jump on :slightly_smiling_face: (and it's not just the two-factor bit that would potentially cause this, any action that triggers a user update within the subsite would have the potential to clear them previously)

Note: See TracTickets for help on using tickets.