Menu collapse option not saved correctly

[griffinjt]

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.

[SergeyBiryukov]

Related: [21574] (for #21349).

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

Could not reproduce on Windows yet.

[nacin]

Setting to 3.5 for review.

[bpetty]

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.

[SergeyBiryukov]

Could not reproduce in Chrome 21.0.1180.89 on Mac either.

[griffinjt]

Here is my screencast of the bug - ​https://s3.amazonaws.com/tgm/ScreenFlow.mov

[griffinjt]

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.

[griffinjt]

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?

[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)?

[griffinjt]

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.

[bpetty]

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.

[SergeyBiryukov]

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

[SergeyBiryukov]

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.

[daankortenbach]

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...

[nacin]

Moving to 3.6 for investigation.

[lessbloat]

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.

[SergeyBiryukov]

Related: #23817

[azaozz]

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.

[azaozz]

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

[aaroncampbell]

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

[azaozz]

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