WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#4051 closed defect (bug) (fixed)

blogroll not functional for internal links

Reported by: thomask Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.7
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

if you try to add e.g.

#menu

as a link address, or some internal page, it saves it as http:/#menu (add http:/ - http with one dash)

Attachments (4)

4051.diff (3.2 KB) - added by Otto42 8 years ago.
Replace clean_url in _walk_bookmarks with a default filter.
filter_blogroll.patch (940 bytes) - added by mrmist 7 years ago.
refreshed against 10289
4051.2.diff (1.3 KB) - added by scohoust 6 years ago.
4051.patch (590 bytes) - added by hakre 6 years ago.
clean_url() acccepts # against 2.8 trunk

Download all attachments as: .zip

Change History (29)

comment:1 @rob1n8 years ago

  • Keywords reporter-feedback added; blogroll removed
  • Milestone 2.4 deleted

Can you try this with the latest SVN? I believe it was fixed.

comment:2 follow-up: @rob1n8 years ago

Never mind. It doesn't try to see if there's a # at the beginning.

Which brings me to the next question. Why would you want to put a page-specific, internal link into the blogroll?

comment:3 in reply to: ↑ 2 @foolswisdom8 years ago

Replying to rob1n:

Which brings me to the next question. Why would you want to put a page-specific, internal link into the blogroll?

Well, #anchor does not mean page specific. The anchor could be in a header or footer. Seems like a very edge case.

comment:4 follow-up: @Otto428 years ago

Quick question: Why is the blogroll trying to verify the links at all? I would prefer it to not try to check http: or ftp: or mailto: or anything else. Is there any easy way to disable this functionality, to let people put anything that they like in there? Maybe they want to make links to other edge cases, like itms: links or something...

comment:5 in reply to: ↑ 4 ; follow-up: @foolswisdom8 years ago

  • Milestone set to 2.4

Replying to Otto42:

I would prefer it to not try to check http: or ftp: or mailto: or anything else.

+1

comment:6 in reply to: ↑ 5 @foolswisdom8 years ago

Replying to foolswisdom:

Replying to Otto42:

I would prefer it to not try to check http: or ftp: or mailto: or anything else.

+1

Changed my mind already. Should validate, but should support other protocols. People are most likely to expect http:// and only enter www.example.com .

comment:7 @foolswisdom8 years ago

  • Keywords reporter-feedback removed
  • Version set to 2.2

comment:8 @rob1n8 years ago

  • Milestone changed from 2.4 to 2.3
  • Owner changed from anonymous to rob1n
  • Status changed from new to assigned

comment:9 @rob1n8 years ago

  • Status changed from assigned to new

comment:10 @Otto428 years ago

Note for anybody else who wants to know:

The fix for this is in bookmark-template.php. Remove this code from the _walk_bookmarks() function:

if ( !empty($bookmark->link_url) )
	$the_link = clean_url($bookmark->link_url);

And all your Bookmark URLs will then go unmolested.

@Otto428 years ago

Replace clean_url in _walk_bookmarks with a default filter.

comment:11 @Otto428 years ago

  • Keywords has-patch added
  • Priority changed from low to normal
  • Severity changed from trivial to normal

Attached a patch that will replace the clean_url call in _walk_bookmarks with a filter. Also added a default filter to hook that to clean_url. By itself, this patch changes nothing. However, it allows plugins to disable it entirely (my preference) or to just modify that filter and thus replace the $protocols parameter of the call to clean_url. Thus a plugin can add allowed protocols and such as well as simply disabling it. Allows for both choices.

comment:12 @Otto428 years ago

  • Keywords 2nd-opinion added

comment:13 @rob1n8 years ago

  • Owner rob1n deleted

comment:14 follow-up: @rob1n8 years ago

Or change clean_url to accept anchors (#whatever). It's a valid URL, after all.

@mrmist7 years ago

refreshed against 10289

comment:15 @mrmist7 years ago

  • Version changed from 2.2 to 2.7

comment:16 @mrmist7 years ago

  • Milestone changed from 2.9 to 2.8

comment:17 follow-up: @scohoust6 years ago

  • Cc wp@… added
  • Keywords needs-testing added; 2nd-opinion removed

Changed clean_url to accept anchors when it is the first character (which is should be). Added a note in the edit form to show an anchor is acceptable. This could be used in combination with the above patch.

comment:18 @Denis-de-Bernardy6 years ago

This seems weird:

<code>&#35anchor</code>

comment:19 @scohoust6 years ago

I agree it doesn't look great, not sure what the best alternative is. Felt like something needed to be added there otherwise it wouldn't make much sense.

comment:20 @Denis-de-Bernardy6 years ago

oh, I was meaning, isn't it missing a ; at the end of the entity?

@scohoust6 years ago

comment:21 @scohoust6 years ago

Duh, of course.

comment:22 in reply to: ↑ 14 @hakre6 years ago

Replying to rob1n:

Or change clean_url to accept anchors (#whatever). It's a valid URL, after all.

clean_url not really cares about valid urls, clean_urls define which URLs are valid for the function.

comment:23 in reply to: ↑ 17 @hakre6 years ago

Replying to scohoust:

Changed clean_url to accept anchors when it is the first character (which is should be). Added a note in the edit form to show an anchor is acceptable. This could be used in combination with the above patch.

Not in 2.8 Trunk. clean_url() does not check for # aka anchors there.

@hakre6 years ago

clean_url() acccepts # against 2.8 trunk

comment:24 @Denis-de-Bernardy6 years ago

  • Keywords commit added; needs-testing removed

+1 to commit either of the last two patches.

comment:25 @ryan6 years ago

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

(In [11126]) Don't prepend http to fragments. Props scohoust. fixes #4051

Note: See TracTickets for help on using tickets.