WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#21795 closed defect (bug) (worksforme)

Menu collapse option not saved correctly

Reported by: griffinjt Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Administration Keywords: reporter-feedback close
Focuses: Cc:
PR Number:

Description

The menu collapse option is not saving correctly when at a resolution > 900px. When collapsing and them re-expanding the menu, the menu auto collapses on each new page load. It's caused by the script deleting the folded setting but not actually setting the unfolded setting as far as I can tell.

The attached patch works for me.

Attachments (2)

common-r21757.diff (393 bytes) - added by griffinjt 7 years ago.
21795.diff (1.4 KB) - added by lessbloat 7 years ago.

Download all attachments as: .zip

Change History (25)

#1 @SergeyBiryukov
7 years ago

Related: [21574] (for #21349).

According to the IRC chat, appears in Chrome 21.0.1180.89 on Mac.

Could not reproduce on Windows yet.

#2 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.5

Setting to 3.5 for review.

#3 @bpetty
7 years ago

Yeah, it does seem weird since based on the fix, you should be able to reproduce this bug on Windows/Linux, and probably other browsers as well, but I couldn't reproduce the actual bug on WinXP with same version of Chrome (21.0.1180.89), or Ubuntu and Chrome 18 (so I also can't confirm the patch). Still need another tester on Mac though.

@azaozz might be able to confirm the fix quickly though.

#4 @bpetty
7 years ago

  • Cc bpetty added

#5 @SergeyBiryukov
7 years ago

Could not reproduce in Chrome 21.0.1180.89 on Mac either.

#7 @griffinjt
7 years ago

I also reproduced the same bug in Firefox on Mac - went away before r21573 and reappeared at r21575 (and still exists with trunk). I've tested on both local and live with clean installs.

#8 @griffinjt
7 years ago

Regardless of reproducing the error, it still seems like the code is missing updating the user setting to keep the menu unfolded, which is why I created the patch. Is this not the case?

#9 follow-up: @azaozz
7 years ago

Couldn't reproduce in Chrome 21.0.1180.89 on both Mac and Windows either.

Yes, [21574] is the changeset that adds folding/unfolding of the menu for narrow screens. It's a bit tricky as the folding now uses two user settings and has three states: always folded, auto-folding at 900px screen width, always unfolded, and these are set according to the screen width at the moment of clicking the button (that's better than having 3-way switch with radio buttons).

Maybe this can be improved. It could keep the settings completely independent for the folding above and below 900px screen width.

@griffinjt it is possible Chrome shows you cached version of the screen(s)?

#10 in reply to: ↑ 9 @griffinjt
7 years ago

Replying to azaozz:

Couldn't reproduce in Chrome 21.0.1180.89 on both Mac and Windows either.

Yes, [21574] is the changeset that adds folding/unfolding of the menu for narrow screens. It's a bit tricky as the folding now uses two user settings and has three states: always folded, auto-folding at 900px screen width, always unfolded, and these are set according to the screen width at the moment of clicking the button (that's better than having 3-way switch with radio buttons).

Maybe this can be improved. It could keep the settings completely independent for the folding above and below 900px screen width.

@griffinjt it is possible Chrome shows you cached version of the screen(s)?

That seems like a good approach, especially moving forward if some other bug creeps in. It would be a lot easier to isolate that way. I've cleaned out the cache and even tested it from my wife's Mac with the same issue - so I'm not sure what the deal is.

#11 @bpetty
7 years ago

  • Keywords reporter-feedback added; has-patch needs-testing removed
  • Milestone changed from 3.5 to Awaiting Review

Since it's been a while with no action here, I just want to note that as long as there is now 3 developers here that were not able to reproduce this bug (two of them testing with the exact same browser on the same platform), it's not likely that this patch will be applied until we can figure out why this is broken for @griffinjt.

In the video it's obvious that there's a bug here somewhere, but without a single other developer being able to reproduce that same bug, we can't actually confirm that this is the appropriate patch. @azaozz does not appear to think that there's anything wrong with this code, or that the patch is necessary.

@griffinjt, since it has only shown itself on your WordPress installation, you're the only one that can help narrow down why this is happening. If you can at least clarify exactly how you have configured and installed WordPress for this to happen, maybe post your wp-config.php, and confirm that you have a completely clean, new database configured, it would help immensely. Please also ensure that you have all browser extensions disabled while testing as they could be causing this.

#12 @nacin
7 years ago

  • Version changed from trunk to 3.4

#13 @SergeyBiryukov
7 years ago

[21574] is the changeset in question here, so 3.5 is technically the earliest affected version.

#14 @SergeyBiryukov
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed
  • Version changed from 3.4 to trunk

No feedback from the reporter in two months, no one else was able to reproduce the issue.

Feel free to reopen with more information if there's still a problem.

#15 @daankortenbach
7 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I can confirm this issue. Looks like it has something to do with the 3 states of the menu and browser width.

I'm on Chrome for Mac 24.0.1312.57

Can't quite figure out what steps excactly to reproduce but you can see it happening while doing the following: collaps, navigate to other admin page, change window width to < 900px, navigate, uncollapse, change width again, navigate, collapse, etc (mix it up a bit). At some point you'll see it will either stay collapsed or stay uncollapsed and you can't get the other as default if you don't change window width first.

Hope this makes sense...

#16 @nacin
7 years ago

  • Milestone set to 3.6

Moving to 3.6 for investigation.

#17 follow-up: @lessbloat
7 years ago

I wasn't able to replicate the issue as described above. In my mind it should:

  • Stay collapsed if you ever manually click "collapse menu", regardless of whether you refresh the browser or resize the browser (above/below 900px).
  • Otherwise, when in the expanded mode (either by default, or by manually clicking back out of collapsed mode), it should always collapse when the browser goes < 900px, and then re-expand when you go back above 900px.

21795.diff is an attempt to get us there.

@lessbloat
7 years ago

#19 in reply to: ↑ 17 @azaozz
7 years ago

Replying to lessbloat:

...

  • Otherwise, when in the expanded mode (either by default, or by manually clicking back out of collapsed mode), it should always collapse when the browser goes < 900px, and then re-expand when you go back above 900px.

It used to be like that at some point. But always collapsed on small screens (i.e. when the responsive css kicks in) is not good for UX, so added expanding there.

Currently the menu has 3 states: collapsed, auto and expanded that are set with one on/off switch depending on the screen width at the time of setting. This seems to accommodate all users' preferences.

There is a bug (at least in FF) described in #23817 that seems to be causing the reported problem here.

#20 @alexvorn2
7 years ago

  • Cc alexvornoffice@… added

#21 @azaozz
6 years ago

  • Keywords close added

Still can't reproduce. Suspecting it has something to do with the bug/browser differences reported in #23817.

#22 @aaroncampbell
6 years ago

I can't seem to reproduce this one either. I'm on Linux and tested in Chromium 28.0.1500.52

#23 @azaozz
6 years ago

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

Closing as worksforme again, possibly fixed with [24619].

Note: See TracTickets for help on using tickets.