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

Add some more media sites #96

Merged
merged 6 commits into from
Mar 10, 2023
Merged

Conversation

iMattPro
Copy link
Contributor

@iMattPro iMattPro commented Mar 6, 2023

No description provided.

Signed-off-by: Matt Friedman <maf675@gmail.com>
Signed-off-by: Matt Friedman <maf675@gmail.com>
Signed-off-by: Matt Friedman <maf675@gmail.com>
Signed-off-by: Matt Friedman <maf675@gmail.com>
Signed-off-by: Matt Friedman <maf675@gmail.com>
Comment on lines 5 to 10
oembed:
endpoint: https://mastodon.social/api/oembed
scheme: https://mastodon.social/@{@name}/{@id}
scrape:
- extract:
url: "!https://(?'host'[-.\\w]+)/@(?'name'\\w+)/(?'id'\\d+)!"
Copy link
Contributor

@rxu rxu Mar 7, 2023

Choose a reason for hiding this comment

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

@JoshyPHP could you please judge if this part of definition is fully correct, especially the imported part of extract which looks like 'extract'=>['#"url":"https://(?\'host\'[-.\\w]+)/@(?\'name\'\\w+)/(?\'id\'\\d+)"#'] in CachedDefinitionCollection.php.

Copy link

@JoshyPHP JoshyPHP Mar 7, 2023

Choose a reason for hiding this comment

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

At first glance it looked fine but on a second read I realized it was a trick of the eye caused by the YML structure. The regexp I use is #"url":"https://(?'host'[-.\w]+)/@(?'name'\w+)/(?'id'\d+)"#. That code looks like it creates a url attribute inside the extract definition.

I should mention that CachedDefinitionCollection.php is auto-generated from XML files. The XML files are canonical and human-readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So... Like the way I just updated it is how it should be?

Copy link

Choose a reason for hiding this comment

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

That looks like it. If there's anything wrong with it, I can't see it.

Signed-off-by: Matt Friedman <maf675@gmail.com>
@rxu
Copy link
Contributor

rxu commented Mar 8, 2023

Bilibili doesn't load in the post for some reason giving just an empty space (but using the link directly in the browser works fine).
image

Tenor loads with the header (not just a GIF), but that's probably like it should be here.
image

@iMattPro
Copy link
Contributor Author

iMattPro commented Mar 8, 2023

For Tenor it loads the GIF fine for me, the header only appears when you mouse over the GIF for me.

For Bilibili, being a Chinese site, maybe there's some country issue there for you??? The example working for me.
Screenshot 2023-03-07 at 8 02 14 PM

@rxu
Copy link
Contributor

rxu commented Mar 8, 2023

Well, regional issue would be the case for Bilibili but example link loads fine directly in the browser. Also the site works on my side in common. Weird.

@iMattPro
Copy link
Contributor Author

iMattPro commented Mar 8, 2023

Yeah I don't know. I've tried in all 4 browsers and they work here (Edge, Firefox, Chrome and Safari)

@iMattPro iMattPro merged commit cc81e13 into phpbb-extensions:master Mar 10, 2023
@iMattPro iMattPro deleted the updates branch March 10, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants