Web-stories-wp: Remove fallback publisher logo

[swissspidy]

Task Description

If the user doesn't have a publisher logo set, we currently fall back to the WordPress logo.

This might not be what a publisher desires, so we might want to consider not doing that anymore.

The format would fall back to the favicon in this case.

[swissspidy]

@o-fernandez To clarify, there are two fallbacks currently in place:

If there's no publisher logo set by the user, we try to first look for a generic site logo that the user might have set up on their site. If there is one, we try to find a square sized thumbnail of it (might be badly cropped). If there isn't, we just use the original (bad, doesn't meet requirements).

If there is no site logo, we use the WP logo.

I think we need to remove both fallbacks, and rely entirely on the publisher logo from the settings.

Sounds good?

[o-fernandez]

Unfortunately, since we can't rely on the site logo to have the right dimensions or be appropriately cropped, I'd agree that we should remove both fallbacks. @choumx for awareness/thoughts.

[spacedmonkey]

It is worth noting here that, there when the code checks for a custom_logo or site_icon it does check to see if it is the correct size before displaying it. But yes, as the site icon / custom logo was set likely when the site was created, it is unlikely that correct size will have been generated. See #4764 for more details.

[swissspidy]

it is unlikely that correct size will have been generated.

Even if the there is a correct size available, it will be badly cropped and not follow recommendations. Hence my recommendation to drop this.

[spacedmonkey]

It seems pretty straight forward to remove the logic to get custom_logo / site_icon. Just remove these lines

https://github.com/google/web-stories-wp/blob/2ee52da00865897d0688d94c52244f63ab3784e7/includes/Traits/Publisher.php#L131-L141

And removing these lines to get rid of the placeholder also seems simple enough.

https://github.com/google/web-stories-wp/blob/2ee52da00865897d0688d94c52244f63ab3784e7/includes/Traits/Publisher.php#L143-L147

We need to keep the placeholder around for this filter

https://github.com/google/web-stories-wp/blob/2ee52da00865897d0688d94c52244f63ab3784e7/includes/Traits/Publisher.php#L159

Is this a breaking change? As users may expect a publish logo to appear and it not longer does so after this change?

[swissspidy]

Note: this would need to work like \Google\Web_Stories\Story_Renderer\HTML::add_poster_images, removing the amp attribute if there’s no publisher logo

Is this a breaking change? As users may expect a publish logo to appear and it not longer does so after this change?

No, I don't think so. FWIW, we might want to shuffle around some things with the filters anyway.

[spacedmonkey]

I have work on PR for this ticket. See #4787 It is marked as draft. Until we have feedback from product, we can't continue.

[swissspidy]

I think the direction was clear after https://github.com/google/web-stories-wp/issues/4762#issuecomment-702135043?

[spacedmonkey]

@swissspidy Does that mean that we are good to go with #4787?

[swissspidy]

It's not complete yet, is it? Left a comment on #4787