Encountering issues with publisher logo/text attribution

ampproject / amphtml

The AMP web component framework.

https://amp.dev

Apache License 2.0

14.89k stars 3.89k forks source link

Encountering issues with publisher logo/text attribution #38422

Closed coreymasanto closed 1 year ago

coreymasanto commented 2 years ago

@adamshwa posted the following in https://github.com/ampproject/amphtml/issues/38329#issuecomment-1227355999

After following these instructions: The data will come from the 's metadata attributes in the story's document, as described:

For the logo: the entity-logo-src attribute (if provided), otherwise the publisher-logo-src attribute. 2 .For the string of text: the entity attribute (if provided), otherwise the publisher attribute.

When the attribution is clicked, it will navigate the user to a URL. This will come from entity-url attribute if provided, otherwise it will use the story's canonical domain.

It failed to display both logo and text, can you please review if there is bug in the code? I tried to debug this area, and it never managed to add anything to src attr, although something was config. Even tried to add break points (screenshot) and it skipped them.

coreymasanto commented 2 years ago

Hey @adamshwa, sorry to hear about your technical difficulties. I have a couple of questions:

Do you have a link to this player/story that I could take a look at?
With your maybeBuildAttribution_() breakpoints, is the return statement at line 365 being hit?

adamshwa commented 2 years ago

Hi @coreymasanto, thanks for finding time to check this.

https://stories.wnba.com/games/19630-21-08-22-Seattle-Storm-vs-Washington-Mystics.html , it might have GEO restrictions, if so try to use US or IL.
no, it really hard to understand why, but 388 being hit without stopping on my breakpoints on 368,372,376 and 382. I see there is an issue around this.systemLayerEl_, it returns null, so the following code fails to find the selector. wdyt?

coreymasanto commented 2 years ago

https://stories.wnba.com/games/19630-21-08-22-Seattle-Storm-vs-Washington-Mystics.html

You've correctly specified the publisher and publisher-logo-src attributes on the amp-story element, so the attribution logo & text should be visible when your story is viewed on Google Search and Google Discover. The attribution is not expected to show when your story URL is accessed directly.

Is your concern that the story is not displaying in Search/Discover with the publisher logo & text attribution? If so, then I think that the attribution should already be displaying correctly on these Google surfaces and no further work should be required. @newmuis, please chime in here if I'm incorrect.

newmuis commented 2 years ago

Sorry, I've been away for the past few days.

Attribution is intended to show up only in viewers that support the attribution feature. It's unclear to me from this thread exactly which platforms are being targeted, but for the ones that have been mentioned:

I actually do not believe that Google's viewers typically support this. As such, you would not see this particular attribution UI element when viewing the story on Google surfaces (though Google does typically render its own attribution UI separately)
As @coreymasanto mentioned, you will not see this attribution UI when navigating to the story's URL directly.

Despite this, there may be other viewers that do support viewing attribution (for example, you can configure amp-story-player to display attribution if you are using it on your own properties).

erwinmombay commented 1 year ago

Closing issue as stale. but feel free to re-open if you have additional information