oEmbed generates link to an image in the wp-admin directory

[nachtwaechter]

The embedded article contains a link to the WordPress logo in the wp-admin directory, if no website icon is specified explicitly.

For most users, it is not an issue. But some people – like me – prefer to have HTTP authentification on the whole wp-admin directory to harden their installation. For those people, an embedded post or page in social media sites or something similar gives an authentification dialog window in the browser while displaying, which is ugly, unwanted and looks like a bug.

There is a very simple workaround, which I am going to describe in a German posting soon: Just set a website icon, which is then used instead of the default WordPress logo. (Why German? Because it is my native tongue and it is spoken by all of my readers.)

But from my point of view, it is an error in the WordPress core. The error is in the sometimes wrong assumption, wp-admin is a world-readable directory in every installation.

I suggest placing a WordPress logo in the wp-includes directory and using this version in embedded views, allowing for using HTTP authentification in wp-admin.

[dd32]

This is definitely something that should be done.

If it didn't require additional images being added I'd have even suggested it would be 4.4.x material (We'd have to use an externally-hosted image if we wanted to move away from the existing wp-admin image)

[peterwilsoncc]

Replying to dd32:

If it didn't require additional images being added I'd have even suggested it would be 4.4.x material (We'd have to use an externally-hosted image if we wanted to move away from the existing wp-admin image)

I was thinking 4.4.x too, moving the logo files to wp-includes is a relatively minor change.

[dd32]

Replying to peterwilsoncc:

Replying to dd32:

If it didn't require additional images being added I'd have even suggested it would be 4.4.x material (We'd have to use an externally-hosted image if we wanted to move away from the existing wp-admin image)

I was thinking 4.4.x too, moving the logo files to wp-includes is a relatively minor change.

To clarify my reasoning for not including the file move/copy in 4.4.1, is that we don't add new files in point releases to as to make point releases more reliable, it's easy to modify a file, but can't always create one. For background updates we prefer to have the wider coverage of only needing to be able to modify, not create.

[thewanderingbrit]

This patch moves the image to wp-includes/images and fixes the references to them in the embed template.

Thanks for the valuable contribution @nachtwaechter. Much appreciated!

[Otto42]

Instead of moving the image, it should probably be a copy, in case any plugins/themes/other expect it to be in the admin directory and are linking to it there.

[thewanderingbrit]

Agreed. I used the wrong term. I did in fact copy the image. The original remains in place.

[thewanderingbrit]

Revised to include src directory

[thewanderingbrit]

attachment:35322.1.diff​ now correctly includes the src directory, but I notice that, with the diff being created by GitHub from ​these directions, the image isn't included in the diff. diff created from ​this PR.

[peterwilsoncc]

Following comment:3, @dd32 and I had a super quick discussion in slack on 4.4.2 patch without new files. It's in 35322-442.diff​:

• I moved the title HTML to a function to avoid double coding.
• Markup unchanged if site logo is defined.
• If no logo is defined, an empty span is placed on the page with a w-logo background as a data uri.
• Logo is SVG for most, PNG for IE8 & earlier

The IE8/png fallback looks a bit mucky, it might be possible to clean it up.

[swissspidy]

Some notes on the second patch:

• I wanted to create a new function for that as well, though a name like print_embed_site_title() sounds better. No need for an $echo parameter though, just echo.
• The sprintf in that function is wrong. Also, the admin_url( 'images/w-logo-blue.png' ) fallback is not needed since there is always an site icon because of has_site_icon() check
• Tests for that function would be nice.

[peterwilsoncc]

Replying to swissspidy:

Some notes on the second patch:

• I wanted to create a new function for that as well, though a name like print_embed_site_title() sounds better. No need for an $echo parameter though, just echo.
• The sprintf in that function is wrong. Also, the admin_url( 'images/w-logo-blue.png' ) fallback is not needed since there is always an site icon because of has_site_icon() check
• Tests for that function would be nice.

FWIW, I think the second patch is too much for the benefit it provides. As a logo is an image and new files can be created in a major release, patch 1 seems more appropriate.

What are your thoughts?

[dd32]

FWIW I agree that the 4.4.x patch is too big for the benefit it provides. wp-admin is not a secret, and very few sites have HTTP Auth in place. Those sites can either go ahead and assign a site logo or allow that file through.

[swissspidy]

I love the simplicity of 35322.1.diff​. Something like 35322-442.diff​ can be looked at as part of #34561.

We should go with the former for 4.5, as it's a relatively minor change with just 1 new file.

[swissspidy]

In 36635:

Embeds: Load the default site icon from the wp-includes directory.

Files inside the wp-admin directory may not be publicly available. This copies the blue WordPress logo to wp-includes/images.

Props thewanderingbrit.
Fixes #35322.