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

DPL3 doesn't support externallinks table changes on 1.41 #274

Open
prodigion opened this issue May 22, 2024 · 9 comments
Open

DPL3 doesn't support externallinks table changes on 1.41 #274

prodigion opened this issue May 22, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@prodigion
Copy link

DynamicPageList3 version: 3.5.3
MediaWiki version: 1.41.1
PHP version: 7.4.3

List of steps to reproduce (step by step, including full links if applicable):

  • Use linkstoexternal in a query

What happens?:
Fatal exception of type "Wikimedia\Rdbms\DBQueryError"
Error 1054: Unknown column 'el.el_to' in 'field list'

@prodigion prodigion added the bug Something isn't working label May 22, 2024
@prodigion
Copy link
Author

This is the change that dropped the el_to column: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/954900

@peterdevpl
Copy link
Contributor

I don't think it's possible for linkstoexternal to work on MW >=1.41. This is because now MediaWiki:

  1. splits URLs into separate columns: domain and path
  2. the domain is reversed to optimize queries, so example.com becomes com.example

It won't be possible anymore to do queries like %example.com/test%.

See https://www.mediawiki.org/wiki/Manual:Externallinks_table and https://www.mediawiki.org/wiki/Help:Linksearch . The latter page lists types of supported queries.

@Universal-Omega
Copy link
Owner

I am looking into a way to fix this but it indeed seems it may not work.

@Universal-Omega
Copy link
Owner

linkstoexternal has just been disabled (by placing it in a 5th richness level) for now.

@Routhwick
Copy link

Routhwick commented Feb 7, 2025

@Universal-Omega: To reverse the "com.example" fragment back into "example.com", one might try to look into this September 2014 StackOverflow solution involving PHP's explode() (i.e. explode('.', $str), where $str = "com.example"). Any chance you might give this workaround a shot?

@Universal-Omega
Copy link
Owner

@Universal-Omega: To reverse the "com.example" fragment back into "example.com", one might try to look into this September 2014 StackOverflow solution involving PHP's explode() (i.e. explode('.', $str), where $str = "com.example"). Any chance you might give this workaround a shot?

This is one idea I've looked at yes but it's a little complex with this extension. How much is the use case for linkstoexternal? Is it actually fully worth it?

@Routhwick
Copy link

@Universal-Omega: How much is the use case for linkstoexternal? Is it actually fully worth it?

Highly doubtful I'm aware of any working Miraheze examples at this point, but it might not hurt to look around in the long run. Once it gets recoded for 1.43+, I'll test it out myself. Until then, there's always FollowTheScore's guide. Good luck!

@peterdevpl
Copy link
Contributor

peterdevpl commented Feb 10, 2025

@Routhwick @Universal-Omega we can use the MediaWiki's LinkFilter::makeIndexes(), I think it's the most reliable solution as it's used to store external links in the database.

However, I'm still thinking about how to handle linkstoexternal conditions with percent signs. It's easy to handle a full URL - just use the function I mentioned - but in DPL, if we have a condition like %foobar%, we don't know where to look for that string. Should we match a domain, path or both? The linkstoexternal behavior will change anyway.

Another case is %example.com/foo%. We can split this one into domain and path criteria, so we end up with com.example% for the domain part and /foo% for the path. However, I'm afraid there's some edge case we can trip over if we process URLs ourselves instead of using makeIndexes().

@Universal-Omega
Copy link
Owner

@Routhwick @Universal-Omega we can use the MediaWiki's LinkFilter::makeIndexes(), I think it's the most reliable solution as it's used to store external links in the database.

However, I'm still thinking about how to handle linkstoexternal conditions with percent signs. It's easy to handle a full URL - just use the function I mentioned - but in DPL, if we have a condition like %foobar%, we don't know where to look for that string. Should we match a domain, path or both? The linkstoexternal behavior will change anyway.

Another case is %example.com/foo%. We can split this one into domain and path criteria, so we end up with com.example% for the domain part and /foo% for the path. However, I'm afraid there's some edge case we can trip over if we process URLs ourselves instead of using makeIndexes().

@peterdevpl

That's a very thorough analysis here. I appreciate it. It does make a lot of sense, and I think you're absolutely right. I think we may loose some functionality or introduce some sort of bug if we go that route, but I think it may still be the best route even if that is the case. Because in my point of view at least, some functionality is better then no functionality however of course if we can make it totally work that would be best. It's just a little bit tricky to account for all situations in this extension. In addition to linkstoexternal it is also likely that addexternallink and the %EXTERNALLINK% format are also broken.

Unrelated but in addition I've also been reminded lately that %EDITSUMMARY% is still broken as I never finished adding support for the comments migration here. Perhaps I should go back and look at that again as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants