Yoast / wordpress-seo

Yoast SEO for WordPress
https://yoast.com/wordpress/plugins/seo/
Other
1.76k stars 885 forks source link

javascript errors break other plugins #3356

Closed senlin closed 8 years ago

senlin commented 8 years ago

As you can see here, when Yoast SEO 3.0.1 is installed and activated the following scripts throw a tantrum:

This breaks other plugins too.

A development team of a plugin that has more than 1mio active installs should in my opinion be a lot more careful when releasing new versions of the plugin. It's one thing that your plugin breaks, but it's a whole different story when you break the SEO of others and when the breakage of your plugin, breaks other plugins too!

senlin commented 8 years ago

3.0.2 does NOT fix this

senlin commented 8 years ago

3.0.3 does NOT fix this

Rarst commented 8 years ago

Please include specific details in your original post, "throw tantrum" is unfortunate, but not informative and easy to act on.

Error messages verbatim are helpful. Also you can switch to non-minified versions by declaring SCRIPT_DEBUG core constant in your configuration, that would make it easier to pin down issues.

senlin commented 8 years ago

oh so now I have to help you guys solving this mess? nah, thanks, but no thanks.

on top of that I linked to the post with details, you can click, right?

funkatron82 commented 8 years ago

@senlin They have to know what's broken in order to fix it. @Rarst Hello Rarst, I'm a contributor for the Meta Box plugin, which is a dependency of @senlin's plugin. I get the following error, which breaks any JavaScript that comes after: wp-seo-post-scraper-302.min.js:1 Uncaught TypeError: Cannot read property 'value' of null

yoasterror This is from the version of Yoast SEO currently available from the WP repository. Hope this helps.

funkatron82 commented 8 years ago

Tried 3.03, same issue. Can't tell what line is the problem because it's minified.

seripap commented 8 years ago

The variables weren't being nullchecked if the metabox was not rendered before the PostScraper.getDataFromInput was called.

funkatron82 commented 8 years ago

Still getting the error

seripap commented 8 years ago

@funkedgeek Of course you are- it hasn't been merged in yet.

funkatron82 commented 8 years ago

I downloaded your fork and I'm still seeing the same error

seripap commented 8 years ago

What's your exact issue? Can you step in the code and show where your error is? The fix I put in was for null checking the post scrapper.

Rarst commented 8 years ago

oh so now I have to help you guys solving this mess? nah, thanks, but no thanks. on top of that I linked to the post with details, you can click, right?

You don't have to do anything. If you would like to see issues resolved faster that's what it takes, simple as that.

Rarst commented 8 years ago

@funkedgeek any chance you can narrow it down to specific Meta Box fields? Or it happens in any case as long as its enabled?

Could you please try with SCRIPT_DEBUG constant enabled in configuration? That will load non-minified versions of the scripts and will help narrow errors down.

seripap commented 8 years ago

I've added more nullchecks and fixed another issue where tinyMCE...getContent returns null if the editor isn't there. @funkedgeek can you confirm your stuff is fixed in this commit 35de84b?

funkatron82 commented 8 years ago

If I'm downloading the right branch, the old errors I posted above are now replaced with errors for TinyMCE. The error is on line 193 of wp-seo-post-scraper-302.js?ver=3.0.3:193 yoasterror yoast2

seripap commented 8 years ago

@funkedgeek do a pull and let me know if that fixes it.

funkatron82 commented 8 years ago

Okay, no more errors. Thank you.

seripap commented 8 years ago

Cool- if the Yoast metabox is missing from the DOM for any apparent reason, it could also throw an error. I've opened a PR on the JS repo to fix this issue, but you can always replace js/dist/yoast-seo-302.min.js with my fork if you need an immediate fix.

senlin commented 8 years ago

@seripap I copied the code from the fork, but still doesn't work. Also it seems that your PR has not been merged yet?

seripap commented 8 years ago

@senlin what's your error?

senlin commented 8 years ago

@seripap my error is the blocking javascript error where I already referred to above.

My plugin is depending on the Meta Box plugin and when Yoast SEO is activated and both Meta Box and my SO Simple Gallery plugins are activated the Add media button does not show anymore, due to the fact that Yoast 3.0.3 throws blocking javascript errors.

Please see below a screenshot with before/after, hopefully that makes it a bit more clear... javascript-error

seripap commented 8 years ago

@senlin did you replace js/dist/yoast-seo-302.min.js with the version in my fork?

senlin commented 8 years ago

Yes, as a matter of fact I did and that didn't change anything. Same errors as you can see from the screenshot below:

your-version
seripap commented 8 years ago

If you run this block of code, does it return null? If so, the fix was merged in and should be in 3.0.4 release. Check out that branch and hopefully it fixes your issue.

document.getElementById( 'content' ) && document.getElementById( 'content' ).value
senlin commented 8 years ago

I'm sorry, how do I "run" that code?

senlin commented 8 years ago

Downloaded 3.0.4, which fixes the issue.

Thanks for your perseverance in getting to the bottom of this Daniel (@seripap)!

Cheers, Piet

WraithKenny commented 8 years ago

Just ran into this, glad it'd be fixed soon

meaelien commented 8 years ago

The problem occurs on version 3.0.7 when I'm hiding the seo meta box

Uncaught TypeError: Cannot read property 'value' of null /wp-content/plugins/wordpress-seo/js/wp-seo-term-scraper-305.min.js?ver=3729b6ccd45e4c5a64a5bb3dac7be717:1

timocouckuyt commented 8 years ago

Just ran into this bug also (on the one in wp-seo-term-scraper-305.min.js), on the term edit screen. With ACF fields (4.4.5) and WP 4.2.6.

Really anoying for clients...

rasmustaarnby commented 8 years ago

This issue still persists on version 3.0.7.

It's quite easily reproduced by going to wp-admin/admin.php?page=wpseo_titles#top#taxonomies and hide the Yoast SEO box on any taxonomy page.

The error: wp-seo-term-scraper-305.js:74 Uncaught TypeError: Cannot read property 'value' of null

Notice that the error has been fixed for post scraper in #3439 but this the issue still persist for the term scraper.

adamwalter commented 8 years ago

This is still a thing in 3.3.1.

screen shot 2016-06-20 at 3 23 11 pm