WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#30214 closed enhancement (wontfix)

Menus: mailto links in menus should be obfuscated

Reported by: ckoerner Owned by: celloexpressions
Milestone: Priority: normal
Severity: normal Version: 4.1
Component: Menus Keywords: has-patch close
Focuses: Cc:

Description

The snazzy Social Media Links feature in Twenty Fifteen allows you to add a naked mailto: link. It should be protected from spam bots and other nefarious crawlers looking for juicy email addresses to consume. Perhaps using antispambot in some capacity.

Attachments (4)

30214.diff (1.2 KB) - added by tywayne 3 years ago.
30214.1.diff (1.2 KB) - added by tywayne 3 years ago.
Fixes typo in last parameter - function accepts only 3 args, not 4
30214.2.diff (1.0 KB) - added by tywayne 3 years ago.
30214.4.diff (668 bytes) - added by tywayne 3 years ago.

Download all attachments as: .zip

Change History (31)

@tywayne
3 years ago

#1 @tywayne
3 years ago

  • Keywords has-patch added

#2 @SergeyBiryukov
3 years ago

  • Component changed from Menus to Bundled Theme
  • Summary changed from mailto: links in Twenty Fifteen's Social Media Links should be obfuscated to Twenty Fifteen: mailto: links in Social Media Links should be obfuscated

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-themes by iamtakashi. View the logs.


3 years ago

#5 @iandstewart
3 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 4.1
  • Type changed from defect (bug) to enhancement

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


3 years ago

@tywayne
3 years ago

Fixes typo in last parameter - function accepts only 3 args, not 4

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


3 years ago

#8 @iandstewart
3 years ago

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

In 30270:

Twenty Fifteen: use antispambot to obfuscate email adresses in the social links menu.

Props tywayne, fixes #30214.

#9 @philiparthurmoore
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Curious why this is limited to social only and not all menus in the theme? Seems to me that it shouldn't matter if the menu is social or primary, the obfuscation would be helpful. So maybe just ditch if ( 'social' == $args->theme_location ) { entirely?

@tywayne
3 years ago

#10 @iandstewart
3 years ago

Philip's question brings up a larger point. Yes, we should be doing this for all menus. At which point, the question becomes, "should a theme do that?" And the answer is no, a theme shouldn't do that. A plugin, or WordPress should do that.

#11 @iandstewart
3 years ago

Yes, only 38 minutes after committing I'm considering removing this from the theme entirely.

#12 @celloexpressions
3 years ago

Shouldn't we add this to core, rather than just doing it in Twenty Fifteen? Would you ever want a bare mailto link in a menu item? Most users won't even know that this can be an issue.

#13 @philiparthurmoore
3 years ago

I would love to see this added into core and not the theme.

#14 follow-up: @iamtakashi
3 years ago

It's a good idea to add to core but I assume it will take time and it might not be in by the time the theme released. So, until then can we add it to the theme for both navigations?

#15 @celloexpressions
3 years ago

  • Component changed from Bundled Theme to Menus
  • Keywords needs-patch added; has-patch needs-testing removed

Oops, said the same thing at the same time :) Let's get a patch that moves this from the theme to core's menus system, for 4.1 to coordinate with Twenty Fifteen's implementation. Shouldn't be much effort required there since we already have the functional part working.

#16 in reply to: ↑ 14 @iandstewart
3 years ago

Replying to iamtakashi:

It's a good idea to add to core but I assume it will take time and it might not be in by the time the theme released. So, until then can we add it to the theme for both navigations?

Well, it is up to the user whether or not they put an email out there. And there are not-WordPress solutions to email harvesting. That is, there are junk mail filters. It's still not defect territory.

#17 @iandstewart
3 years ago

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

In 30273:

Twenty Fifteen: Removing email obfuscation from social links menu.

We really should be doing this in all menus, at which point this becomes core or plugin functionality, not a theme feature.

Closes #30214.

#18 @iandstewart
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Summary changed from Twenty Fifteen: mailto: links in Social Media Links should be obfuscated to Menus: mailto links in menus should be obfuscated

#19 @iandstewart
3 years ago

  • Owner changed from iandstewart to celloexpressions
  • Status changed from reopened to assigned

#20 @tywayne
3 years ago

This could be a little more complex as a core implementation. Should we be considerate of someone already using the nav_menu_link_attributes filter to encode a mailto: href? If core encodes by default, existing theme/plugin code could double encode.

#21 @celloexpressions
3 years ago

Not sure that we could really do much to counteract that, but unless there are other reasons not to encode by default, I think we should, and work on transitioning any plugins that may be using it, or trying to detect that in core. We'd also probably want to put this directly in the menus system rather than adding via a filter, most likely doing so in wp_update_nav_menu_item().

#22 @tywayne
3 years ago

Sounds good to me. And I agree on not adding it via a filter.

Though, if we do it in wp_update_nav_menu_item() it will only get applied when the menu is initially created or on save. If we wait until right before the nav_menu_link_attributes filter in the Walker_Nav_Menu class, it would catch and encode any existing mailto: links without having to re-save the menu.

Since it is essentially a formatting issue, I wouldn't think we'd need to store the encoded link in the menu item, we just need to encode during output.

#23 @celloexpressions
3 years ago

Oh yeah, should definitely be on output, not when saving. Good call.

@tywayne
3 years ago

#24 @dd32
3 years ago

I just want to point out that this function hadn't actively been used by core in 11 years - when things were still in files labeled b2.

Obfuscating the email address isn't going to increase security of the email, and although admittedly it'll stop some old email scrapers, as soon as WordPress is actively using the function on such a large portion of the web, scrapers will just fetch the obfuscated email and easily convert it back.
WordPress has always avoided security by obscurity, just to stop people thinking they're safer than they actually are.

#25 @celloexpressions
3 years ago

  • Keywords has-patch close added; needs-patch removed

Good point from @dd32 - if 23% of the internet starts encoding emails this way, bots will be adapted to look for this. Anyone have any other opinions or proposed solutions other than obfuscation?

#26 @johnbillion
3 years ago

  • Milestone 4.1 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

Yeah, this is a futile exercise these days. HTML entities can't save us now.

#27 @nacin
3 years ago

I am very glad to see this ticket resolved this way. I will say that antispambot() *is* used in core, as a filter callback. I want to note this in case anyone later discovers that and then this ticket — that knowledge should not change the outcome of this ticket.

Note: See TracTickets for help on using tickets.