Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 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
10 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
10 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
10 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
10 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
10 years ago

disregard/deleted. Invalid solution.

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

#6 @AITpro
10 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 10 years ago by AITpro (previous) (diff)

#7 @dd32
10 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
10 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
10 years ago

Well spotted.

#10 @AITpro
10 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 10 years ago by AITpro (previous) (diff)

#11 follow-up: @AITpro
10 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 10 years ago by AITpro (previous) (diff)

#12 @AITpro
10 years ago

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

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

#13 in reply to: ↑ 11 @dd32
10 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
10 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
10 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 instead of doing anything at all. ;) Very odd that this would work at all, but it does for whatever reason that may be. My knowledge of js is very limited. ;)

.href.location.hash
Version 2, edited 10 years ago by AITpro (previous) (next) (diff)

#16 @AITpro
10 years ago

Invalid disregard/deleted.

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

#17 @AITpro
10 years ago

Invalid disregard/deleted.

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

#18 @AITpro
10 years ago

Invalid disregard/deleted.

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

#19 @AITpro
10 years ago

Invalid disregard/deleted.

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

#20 @AITpro
10 years ago

Invalid disregard/deleted.

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