Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#31758 closed defect (bug) (fixed)

hashtag stripped out of wp-admin backend Dashboard URL's

Reported by: aitpro's profile AITpro Owned by: dd32's profile dd32
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.2
Component: Administration Keywords:
Focuses: Cc:

Description

Not sure if this is new intended behavior or something that may be a bug.

WP Version: WordPress 4.2 Beta2
Test Example: /wp-admin/index.php#test
Result: #test is stripped from the URI
Other Example: div id created. hashtag on end of the URL pointing to that div id.
Result: hashtag is stripped from the URL resulting in the link not going to the intended div id.

Change History (20)

#1 @AITpro
9 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Argh never mind. The solution is to escape the hashtag. Sorry.

#2 @AITpro
9 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Oops had 2 test sites open at the same time and thought escaping the hashtag worked, but it helps to be looking at the correct site.

So original issue is still occurring with hashtags being stripped out of all wp-admin URL's.

#3 @AITpro
9 years ago

The problem code is here: /wp-admin/includes/misc.php
Code Line: 854
Function name: wp_admin_canonical_url

Commenting out this action - hashtags are no longer stripped out of wp-admin URL's

add_action( 'admin_head', 'wp_admin_canonical_url' );

#4 @AITpro
9 years ago

And specifically with this js code:

if ( window.history.replaceState ) {
			window.history.replaceState( null, null, document.getElementById( 'wp-admin-canonical' ).href );
		}

#5 @AITpro
9 years ago

disregard/deleted. Invalid solution.

Last edited 9 years ago by AITpro (previous) (diff)

#6 @AITpro
9 years ago

I'm having one of those "special moments" days. Jeez how to not see something that is so freakin obvious.

The Solution:

window.history.replaceState( null, null, document.getElementById( 'wp-admin-canonical' ).href.location.hash );

Just some side notes for anyone else having a "special moment" like me: Since fragments are not something stored server-side and are client browser side only then you cannot do things like parse or use stored server variables. Yeah basic stuff, but I totally spaced out for a bit and forgot that basic stuff.

Last edited 9 years ago by AITpro (previous) (diff)

#7 @dd32
9 years ago

  • Component changed from General to Administration
  • Milestone changed from Awaiting Review to 4.2
  • Version set to trunk

Shifting for review

#8 @dd32
9 years ago

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

In 31882:

When altering the admin URL to reflect the canonical location, keep the existing hash (if present) in the URL.
Fixes #31758. See #23367

#9 @johnbillion
9 years ago

Well spotted.

#10 @AITpro
9 years ago

Yup could have been a pain in the ass if it managed to get all the way through dev to prod. Not sure why I used hashtag and not hash anchor. I was having a very special day - even tying shoelaces was a challenge. :) Note to self: 2 margaritas == fun. 6 margaritas == fubar.

Last edited 9 years ago by AITpro (previous) (diff)

#11 follow-up: @AITpro
9 years ago

@dd32 - good correction on concatenation (.) vs addition (+).
Correction: + is used for concatenation and addition in js. The dot was just a mistake on my part and + was intended.

Last edited 9 years ago by AITpro (previous) (diff)

#12 @AITpro
9 years ago

@johnbillion - didn't say thanks. So thanks. My pleasure to help in any small way that I can.

Last edited 9 years ago by AITpro (previous) (diff)

#13 in reply to: ↑ 11 @dd32
9 years ago

Replying to AITpro:

@dd32 - good correction on concatenation (.) vs addition (+).

Ah! I looked at your suggested code, but didn't catch the direction you were trying to go in (I thought you were trying to access the .href.location.hash property, which obviously doesn't exist!).

Glad to see we ended up at the same basic patch though, had I have realised the minor error, I'd have given you props in the commit, will have to manually add the props later :)

#14 @AITpro
9 years ago

Yeah for whatever reason in js (which should not be allowed) the dots work to "add" vars and other things - ie x.y.z, but it is a very bad coding practice to say the least. I just slapped that together to give the needed info and moved onto other problems without really refining anything. Tested it, it worked, moved on to other stuff.

Not concerned about props whatsoever. Just glad it is handled. Next...

#15 @AITpro
9 years ago

Correction: "...it is a very bad coding practice to say the least..."
It is completely invalid code,usage so it should throw an error or do nothing instead of doing anything at all. ;) Very odd that this would work at all, but it does for whatever odd reason that may be. ;)

.href.location.hash
Last edited 9 years ago by AITpro (previous) (diff)

#16 @AITpro
9 years ago

Invalid disregard/deleted.

Last edited 9 years ago by AITpro (previous) (diff)

#17 @AITpro
9 years ago

Dumber

Or I do not really understand the purpose for this function at all: wp_admin_canonical_url(). Most likely. :)

  • Remove specific query string parameters from a URL, create the canonical link,
  • put it in the admin header, and change the current URL to match.

So is this header only or header and visual? "change the current URL to match" suggests visual.

Version 2, edited 9 years ago by AITpro (previous) (next) (diff)

#18 @AITpro
9 years ago

Invalid disregard/deleted.

Last edited 9 years ago by AITpro (previous) (diff)

#19 @AITpro
9 years ago

Invalid disregard/deleted.

Last edited 9 years ago by AITpro (previous) (diff)

#20 @AITpro
9 years ago

Invalid disregard/deleted.

Last edited 9 years ago by AITpro (previous) (diff)
Note: See TracTickets for help on using tickets.