WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

Last modified 8 months ago

#24921 closed defect (bug) (fixed)

Once the admin menu is collapsed, the user setting gets stuck on collapsed

Reported by: DrewAPicture Owned by: azaozz
Milestone: 3.6.1 Priority: normal
Severity: normal Version: 3.5
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

Seems like once you collapse the admin menu and the mfold user setting is set, it doesn't ever get unset. I'm seeing this in the 3.6 branch and the 3.6 package shipped today.

I tried to trace it back to any recent changes made but I couldn't find anything. Tested in FF, Chrome and Safari on local and on a server.

Works as expected in trunk.

To reproduce:

  1. In the Dashboard, collapse the admin menu
  2. Refresh. Expected: Collapsed, Actual: Collapsed.
  3. Expand the menu.
  4. Refresh. Expected: Expanded, Actual: Collapsed.

Attachments (6)

24921.patch (756 bytes) - added by SergeyBiryukov 9 months ago.
24921.2.patch (2.1 KB) - added by azaozz 9 months ago.
24921.3.patch (7.0 KB) - added by azaozz 8 months ago.
24921.4.patch (7.1 KB) - added by azaozz 8 months ago.
24921.diff (15.7 KB) - added by nacin 7 months ago.
24921.2.diff (15.6 KB) - added by nacin 7 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 knutsp9 months ago

Tried to reproduce this with Firefox 22 and Chrome 28, on both LAMP and WIMP installations running WP 3.6 (auto upgraded from 3.5.2). Could not reproduce.

comment:2 azaozz9 months ago

Works properly here. The only recent change to this was in [24619] (#23817) as WebKit calculates the body width differently for @media css rules when there is a vertical scrollbar.

Perhaps try getUserSetting('mfold') and deleteUserSetting('mfold') from the console at different window widths.

comment:3 ocean909 months ago

Could reproduce it on a clean install. The setting is cleared from the cookie, but get_user_setting('mfold') in admin-header.php still returns 'f'.

comment:4 SergeyBiryukov9 months ago

Looks like [21574] brought back #18091.

deleteUserSetting() doesn't work as expected if the current cookie is empty.

SergeyBiryukov9 months ago

comment:5 SergeyBiryukov9 months ago

24921.patch is the same fix as in [19091].

comment:6 helen9 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.6.1

azaozz9 months ago

comment:7 azaozz9 months ago

24921.2.patch fixes updating the user option when the cookie is empty. May not be suitable for a dot release as it changes the behaviour a bit, but it will fix all user settings not just 'mfold'.

In the meantime a workaround for new installs is to change another user-state setting, for example resize the editor or switch Visual => Text and back.

Last edited 9 months ago by azaozz (previous) (diff)

azaozz8 months ago

comment:8 azaozz8 months ago

24921.3.patch​ is same as 24921.2.patch​ plus:

  • Use get_current_user_id() instead of wp_get_current_user() as only the user ID is needed.
  • Use is_user_member_of_blog().
  • Some cleanup of var names, erc.

azaozz8 months ago

comment:9 azaozz8 months ago

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

In 25109:

User state settings:

  • Allow empty cookie value to be saved.
  • Use get_current_user_id() instead of wp_get_current_user().
  • Use is_user_member_of_blog().
  • Sanitize both name and value in wp_set_all_user_settings().
  • Some var names and code formatting cleanup.

Fixes #24921.

comment:10 helen8 months ago

I had put this in 3.6.1 but I guess it was introduced in 3.5?

comment:11 azaozz8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Yeah, was just going to reopen it for 3.6.1. This hasn't been working properly for ages, introduced probably in 2.8-2.9.

It makes difference only on brand new installs. As soon as the user switches Visual => Text or resizes the editor, the cookie is not empty any more and folding/unfolding works as expected. Not sure if a subset from the patch should go in 3.6.1. It's not a regression in 3.6.

comment:12 SergeyBiryukov8 months ago

  • Version changed from 3.6 to 3.5

It was a regression in 3.5, see comment:4.

comment:13 nacin8 months ago

  • Keywords commit added; dev-feedback removed

Let's do 24921.patch for 3.6.1. Per IRC with duck_, "Not a 3.6 regression, but it only affects new installs so would be good to have fixed in the lastest package".

comment:14 SergeyBiryukov8 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 25185:

Fix menu folding on new installs. fixes #24921 for 3.6.

comment:15 azaozz8 months ago

In 25186:

Fix folding/unfolding of the admin menu on new installs, props SergeyBiryukov, fixes #24921 for 3.6.1.

comment:16 azaozz8 months ago

[25186] adds the fix from [25185] to trunk to keep it in sync.

nacin7 months ago

nacin7 months ago

Note: See TracTickets for help on using tickets.