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

Wordpress detection fails #203

Open
connorjclark opened this issue May 3, 2023 · 6 comments
Open

Wordpress detection fails #203

connorjclark opened this issue May 3, 2023 · 6 comments

Comments

@connorjclark
Copy link
Contributor

For wordpress.com, which I assume it built using wordpress... this library fails to detect it.

cc @housseindjirdeh @westonruter

@westonruter
Copy link

Which URL on wordpress.com? On https://wordpress.com/?

@connorjclark
Copy link
Contributor Author

Yeah, the home page... but only w/ mobile emulation enabled in devtools.

@westonruter
Copy link

On desktop, if not logged-in to WordPress.com, I see:

<meta name="generator" content="WordPress.com">

I see the same when emulating as Chrome Android Mobile.

(When I'm logged into WordPress.com I'm redirected to the admin backend of a site I manage there.)

When I look at an actual blog hosted on WordPress.com, like vintageportland.wordpress.com, I see the same generator tag. I'm actually a bit surprised that "WordPress.com" is the generator rather than the normal "WordPress". Nevertheless, both would seem to work fine according to the current library detection logic:

const generatorMeta = document.querySelector('meta[name=generator][content^="WordPress"]');
const version = generatorMeta ? generatorMeta.getAttribute("content").replace(/^\w+\s/,'') : UNKNOWN_VERSION;
return { version };

When I run that logic in DevTools, it is resulting with a version being "WordPress.com".

So that would seem to be working correctly?

@connorjclark
Copy link
Contributor Author

image

Perhaps there is an A/B test? For me, on desktop hasWPIncludes is true (the other is false); but under mobile emulation hasWPIncludes is false. There is no wlwmanifest file for the mobile load (<link rel="wlwmanifest" type="application/wlwmanifest+xml" href="//s1.wp.com/wp-includes/wlwmanifest.xml">)

@westonruter
Copy link

Oh, I passed over those parts of the test:

const hasAPILinkElem = !!document.querySelector('link[rel="https://api.w.org/"]');
const hasWPIncludes = !!document.querySelectorAll('link[href*="wp-includes"], script[src*="wp-includes"]').length;
if (!hasAPILinkElem && !hasWPIncludes) return false;

I'm somewhat surprised those conditions are part of the test. Someone can turn off the WordPress API, and thus hasAPILinkElem would be false. Similarly, a theme doesn't necessarily include any scripts or stylesheets that load from wp-includes (especially if an optimization plugin is processing them).

Also, it turns out that the wlwmanifest.xml link is deprecated as of 6.3.0 (the next stable release) and will be removed from WordPress core. So it is expected that this wouldn't show up in the future:

<link rel="wlwmanifest" type="application/wlwmanifest+xml" href="https://example.com/wp-includes/wlwmanifest.xml">

So the link[href*="wp-includes"] selector will increasingly become unreliable for WordPress detection going forward. WordPress.com usually runs bleeding edge WordPress (pre-stable), which is perhaps why it is not showing up there now.

I suggest that the hasAPILinkElem and hasWPIncludes conditions be removed.

@westonruter
Copy link

It looks like originally when the test was introduced (2ab612a), it was checking for the meta[name="generator"] tag or the page having a script/link that mentions wp-includes. This was then changed in a8a6276 to make it a both/and condition. See #131 (review).

Pull request in question: #131

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

No branches or pull requests

2 participants