WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 9 years ago

#13994 closed defect (bug) (fixed)

Menu Item "Home" won't have class "current-menu-item" in case of there are query string in the home url

Reported by: kamiyeye Owned by: filosofo
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: has-patch
Focuses: Cc:
PR Number:

Description (last modified by dd32)

Repro steps:
==========

  1. Add "Home" to the navigation menu
  2. Navigate to http://thetestsite/?style=stylename[[BR]]
  3. Check the css class of "Home" on the navigation menu


Expected:
==========
There is the class "current-menu-item"

Actual:
==========
There isn't the class "current-menu-item"

Desc:
==========

  1. It works when there are no query string in the url say: http://thetestsite/[[BR]]
  2. It is related to Changeset 14457

Attachments (2)

front-page-current-class.13994.diff (886 bytes) - added by filosofo 10 years ago.
front-page-current-class.13994.2.diff (1.1 KB) - added by filosofo 9 years ago.

Download all attachments as: .zip

Change History (17)

#2 @kamiyeye
10 years ago

Repro steps:
==========

  1. Add "Home" to the navigation menu
  2. Navigate to http://thetestsite/?style=stylename
  3. Check the css class of "Home" on the navigation menu


Expected:
==========
There is the class "current-menu-item"

Actual:
==========
There isn't the class "current-menu-item"

Description:
==========

  1. It works when there are no query string in the url say: http://thetestsite/
  2. It is related to Changeset 14457

#3 @dd32
10 years ago

  • Component changed from General to Menus
  • Description modified (diff)

#4 follow-up: @filosofo
10 years ago

How would you propose that WP go about determining which query strings make a significant-enough change to warrant saying the page is no longer the same? In this example presumably the only change has to do with the active stylesheet, but most query strings change the content.

I propose that any plugins or themes that use these strings take on the responsibility of handling the menu classes, as they're in the best (really--the only) position of being able to make that decision.

#5 in reply to: ↑ 4 @kamiyeye
10 years ago

Query based on request url should have happened this time, can we use query results or relevant methods to handle this problem? is_home()?

#6 @filosofo
10 years ago

The problem is not identifying the current query; it's matching it to the menu item's URL.

Menu item's URL: http://site.com/

Requested page: http://site.com/?foo=bar

How should WP know that the second is supposed to be equivalent to the first?

#7 @kamiyeye
10 years ago

I think we are talking about the possibility of resolve this issue, right? Any reason we have to stick to the way by comparing url?
Have a try with is_home() on http://testsite/?que=ry, see true or false.

#8 @filosofo
10 years ago

  • Keywords has-patch added
  • Milestone changed from 3.0.1 to 3.1

Thinking about this some more, I think this could be useful for matching the home URL when pages are involved, too.

Try front-page-current-class.13994.diff to see if that solves the problem for you.

#9 @kamiyeye
10 years ago

Cool, it works! Will it be added to wp version 3.0.1?

#10 @filosofo
10 years ago

  • Owner set to filosofo
  • Status changed from new to accepted

#11 @filosofo
9 years ago

  • Milestone changed from Awaiting Triage to 3.1

#12 @filosofo
9 years ago

Patch is still fresh.

#13 @westi
9 years ago

When is this code meant to trigger?

http://trunk.domain/?foo=bar which seems to be what the ticket is talking about doesn't trigger it.

#14 @filosofo
9 years ago

Apparently someone's been untrailingslashitting stuff in the last half year.

Patch refreshed.

#15 @markjaquith
9 years ago

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

(In [16742]) Highlight home page Custom Nav Menu item even when extra query args are present in the URL (only if all other highlights fail). props filosofo. fixes #13994

Note: See TracTickets for help on using tickets.