(Sub-)domain detection not reliable.

We have javascript testing now.

So if you wanted to write a test and try integrating psl, it would be appreciated.

If Nebel54 isn't interested, I'd like to try writing a test for this.

Public Suffix list is 120kb. Where as our javascript is 9kb. I don't really want to add that overhead.

I've attached some related issues.

I'm adding some expanded testing of domains. We should build a list of domains to test conditions with.

@angelamnr, I'd love your help/input on this.

Code standard fixes.

We already include an option for including/excluding via regex, which could be used to work-around this problem.

In your case you could work-around the problem with an exclude pattern such as wwf\.ch\/

Though I can see that as being potentially difficult for many users.

Because it's literally impossible for us to determine true "subdomains" without making a massive whitelist (and even then it's difficult), I would suggest we make another simpler setting for "Consider these domains to be internal" setting, where you can enter one domain name per line. Combining with "Include sub-domains as internal", then you could effectively tell External Links what your top level domain is and have it deal with subdomains without listing them all individually (though you could just list subdomains individually as well).

So in your case, you could simply add one line to the internal domains list: wwf.ch. Check the box for "Exclude links with the same primary domain" and then you'd be set.

External links aims to simplify a strangely thorny problem, but if we make it so inefficient/heavy that it impacts performance people would end up finding custom solutions to be more tenable. It's already much, much heavier than a custom solution be and we should try to avoid impacting performance further.

So if simplifying the configuration would make this easy enough, then that's a preferable route. Or if the regex options are already considered suitable, perhaps we should just continue to suggest that when such problems occur.

Thanks quicksketch, I think in the future we will try and improve the regex along with the tests.

The work around I think could probably be better communicated. We might need to update the text against the "Exclude links with the same primary domain" in the extlink configuration section.

I also have a feeling that Nebel54 was actually using drupal 7 and not 8.

A fix was recently done to the regex for drupal 7 to fix it #2962335: External link icon not always shown after update to 7.x-1.19.

Since the issue for drupal 7 was in #2962335: External link icon not always shown after update to 7.x-1.19, I will close this issue.

Also, love love love WWF, so glad to see our work in use there. :)

Thanks Nebel54! I appreciate the information.

This patch fixes links if they have a port.

https://jsfiddle.net/yqmgbbd0/31/

I did not mean to reopen this. As @elachlan maintains the D8 version I'll leave this to him.

I think the solution offered by Nebel54 makes the most sense.

It might require that the regex portion is refactored out into its own set of functions.

This would also make it easier to write tests for to refine the regex.

I agree. I think there is too much variation possible to do it the way it currently works and have it work for everyone.

If we're worried about the size, potentially there could be a setting to use the old way (I would suggest the the regex from version 7.x-1.18 works for more people than the newer version) or switch to the extra lookup method. So that only users that have a problem with their domain name need activate the extra file.

Or, we just write our own code that uses the list. We possibly don't need all the functionality provided by that parser.

The other option of course, that would work for a large amount of users, who only run a single public facing domain name, is to make people explicitly configure their domain name and sub domains.
But then how do you best handle sites that have a lot of domains or sub domains.

We need to refactor the javascript for easier testing and then use something similar to this:
https://stackoverflow.com/questions/7930751/regexp-for-subdomain

I think we are overthinking this.

If we add a parent domain variable to extlink and make the sub-domain detection opt-in. Then we can use the parent domain for link detection.

We could make it even easier by using PHP PSL and have the setting as a tick box.

The javascript approach will never be foolproof. Since it would need to use PSL to achieve this, which would bloat the module.

I'm adding a patch to address the error from this example:

Link: https://example-comm.marketplace.com; Server: example.com

Currently, the module detects this link as an internal link. Why?

Server URL is example.com. Link is https://example-comm.marketplace.com. The problem is host variable, which its content is added to the RegExp, is 'example.com'. Instead, it should be 'example\.com', because the dot has meaning in a RegExp (it means any char). So, the RegExp engine happily matches 'https://example-com', because that dot matches any char so it matches the '-', and module treats the link as an internal link.

I was not sure whether to add a new issue or not. Because this is strongly related to domain detection and changes the same line as the last patch and issue is still open I thought is ok to add it here, but I can create a new issue if needed.

Does #29 has been tested again the 8.1.x???

The patch still applies, so it should work.

Ok

#32, I'm sorry but I don't understand your request, a screenshot of what? I mean, this issue is related to how external domains are detected, not about any GUI component, so I wouldn't know what to capture on the screenshot. Can you provide more information on what is needed?

@Neslee, this issue is a long running issue on how to improve domain detection. We have investigated Public Suffix List among other things. Additionally testing is difficult because the domain detection code isn't exposed separately. If we were to refactor the code so that the domain detection can be run separately, then we could add the PSL tests to our own test suit.

There is another suggestion of having an administrator enter their domain into the admin panel, so that we can use it when matching. This shortcuts the domain detection. I believe this is the best suggestion because it is explicit and removes the need for PSL, unless the drupal instance is hosting many different domains. I think this covers the majority of the use cases.

For now if you could commit the patch and begin investigating adding a setting to specify a domain in the admin settings that would be fantastic.

We could possibly use a PHP domain parser to store a few variables for the current domain. Something like:
https://github.com/jeremykendall/php-domain-parser

I think this issue is done: patches were committed and no comments complaining were posted.

@tunic this is a long running issue, so we have to keep this open.

@Neslee Canil Pinto maybe we could mark as fixed and continue over at #3358120: not working for subdomain like http://www.xyx.com/my-en