Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YouTube: Fix missing fields when run from translation-server #3293

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

geofferb
Copy link
Contributor

@geofferb geofferb commented Apr 2, 2024

Fixes #3290 by adding an extra fallback for Title, runningTime, Date, Duration, and Description fields based on a regex search for the microformat object, which can be found in the HTML regardless if the page's JS is loaded.

Copy link
Member

@AbeJellinek AbeJellinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I think we can get all this data from page <meta> tags. Title is in <meta name="title">, description is in <meta name="description">, duration is in <meta itemprop="duration">, etc.
  2. We should not use any of these fallbacks if we aren't on the server (Zotero.isServer), because they won't change as you browse around the site and navigate to other videos in the mobile/desktop SPA.

let jsonLD;
try {
jsonLD = JSON.parse(text(doc, 'script[type="application/ld+json"]'));
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert!

@geofferb
Copy link
Contributor Author

geofferb commented Apr 2, 2024

  1. I think we can get all this data from page <meta> tags. Title is in <meta name="title">, description is in <meta name="description">, duration is in <meta itemprop="duration">, etc.
  2. We should not use any of these fallbacks if we aren't on the server (Zotero.isServer), because they won't change as you browse around the site and navigate to other videos in the mobile/desktop SPA.

@AbeJellinek
I started with the Meta tags but switched to the microformat when I realized that the description meta tag only contained a truncated version of the description. If that's okay I'll use the Meta tags for everything.

@AbeJellinek
Copy link
Member

I think it's alright if it's just on the server. Not ideal, but parsing the microformat is way messier, slower (because of the global regex search), and more prone to breakage.

@geofferb geofferb requested a review from AbeJellinek April 5, 2024 04:12
@AbeJellinek AbeJellinek merged commit 82f28ce into zotero:master Apr 11, 2024
1 check passed
@AbeJellinek
Copy link
Member

Thank you!

@geofferb geofferb deleted the youtube branch April 12, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

YouTube: Some fields not found when run from translation-server
2 participants