wp-shortcake / shortcake

Shortcake makes using WordPress shortcodes a piece of cake.
GNU General Public License v2.0
664 stars 143 forks source link

Updating existing shortcode inserts new shortcode #584

Open coderaaron opened 8 years ago

coderaaron commented 8 years ago

As I said in the title, when I edit an existing shortcode and click the "Update" button it inserts a second shortcode with the new attributes rather than updating the existing shortcode.

I thought this might be caused by some of our custom code, but I'm still experiencing the problem on a fresh 4.4.2 install using Twenty Fifteen with only Shortcake 0.7.0-alpha and the Example plugin active.

Apologies if this is a repeat, my search-foo seems to be off this morning.

coderaaron commented 8 years ago

So this appears to be an issue with my Chrome install...apologies

StaggerLeee commented 8 years ago

Do you have any more details about this problem ? I am experiencing this too. Firefox doesnt repeat shortcode.

Needs explanation as this is not so small problem.

coderaaron commented 8 years ago

I figured out that it was only happening when the shortcode was the very first thing in the content (and only in Chrome). To fix I just put a   before the content

khromov commented 8 years ago

Also seeing this issue on Chrome... Can we reopen @danielbachhuber ?

danielbachhuber commented 8 years ago

Also seeing this issue on Chrome

@khromov On master or on the latest tagged release?

khromov commented 8 years ago

@danielbachhuber Latest tagged

danielbachhuber commented 8 years ago

@khromov Can you see if you can reproduce on master?

StaggerLeee commented 8 years ago

I have master files overwritten, believe so.

Just disabled all plugins, except Shortcode UI and my own Shortcake shortcodes plugin. Same problem. And activated default theme to be sure. Same problem.

StaggerLeee commented 8 years ago

Difficult to find right keywords to search about this problem on Google. Only Shortcake and handfull of visual composer plugins use live shortcode updating.

StaggerLeee commented 8 years ago

@coderaaron

I figured out that it was only happening when the shortcode was the very first thing in the content (and only in Chrome). To fix I just put a   before the content

Yes, it is true. Only almost top shortcode makes it. And inserting anything before it removes problem. I just made a test with dev.php code and problem is the same. Stupid me, lost hours changing things in my own code, believing I am doing wrong. Could test with dev.php from first minute.

goldenapples commented 8 years ago

Thanks for the info on how to reproduce. I was able to reproduce the bug on master in Chrome when the shortcode was the first element in the post content. Strangely, I couldn't reproduce it in Firefox. Reopening this issue as a bug.

StaggerLeee commented 8 years ago

Some info to maybe help you in troubleshooting. Only some text before shortcode prevents repeating on "update".

I just tested with native gallery shortcode before Shortcake shortcode and it did not help. Shortcode stil duplicate own shortcode, this time just before gallery shortcode.

You get the point.

goldenapples commented 8 years ago

Yeah, the bug is actually coming from the regex core defines as wp.shortcode.regexp(). We've debated several times whether to use core's shortcode regex or build our own.

The issue is that calling Regexp.exec() in Javascript will start by default from the lastIndex which is set as an internal property of the regex, and only return matches after that character. We fixed an issue related to this a while back in #560 by resetting the lastIndex to 0 before each match.

goldenapples commented 8 years ago

In Firefox console:

> var re = wp.shortcode.regexp('test')
< undefined
> var content = '[test attr="attr"]inner content[/test]'
< undefined
> re.exec(content)
< ["[test attr="attr"]inner content[/test]", "", "test", " attr="attr"", undefined, "inner content", "[/test]", ""]

In Chrome console:

> var re = wp.shortcode.regexp('test')
< undefined
> var content = '[test attr="attr"]inner content[/test]'
< undefined
> re.exec(content)
< null
dmackinn commented 8 years ago

We're seeing this issue as well. Updating will continuously create additional duplicate shortcodes.

khromov commented 8 years ago

@goldenapples I've been tracing this error and end up in the insert function of media-controller.js. That functions just sends the shortcode content to core via send_to_editor()... Could you elaborate where you think the problem lies?

goldenapples commented 8 years ago

I spent a while the other day trying to trace down this error but couldn't pinpoint it. I'd appreciate any advice anyone can provide.

I suspect that there might have to be a fix involving a TinyMCE beforeExecCommand hook, like core does to set the cursor location before inserting an image. But I didn't get deep enough to figure out why the bug is happening or how to fix it.

goldenapples commented 8 years ago

My comments on https://github.com/wp-shortcake/shortcake/issues/593#issuecomment-215999617:

I believe that this bug has been fixed in WordPress core, with the update to TinyMCE 4.3.10. (At least I can't reproduce it on Chrome 50.0.2661.86.)

Editing posts in Chrome with WP 4.4.2, I see the bug described above and in #584. In 4.5 and 4.5.1, the problem doesn't occur as far as I can tell.

Other than the TinyMCE update in 4.5.1, I can't think offhand of a changeset that would have fixed this. But as I'm unable to reproduce in WP 4.5.x, I'd move to close this issue. Anyone?

StaggerLeee commented 8 years ago

Seems as it is OK now. Can confirm it.

BenFausch commented 8 years ago

Hi guys, we're seeing a fix within chrome for this, but now have a client experiencing this exact issue in Win7/IE11 with 4.5.2 that I'm able to reproduce in IE.

Client goes to save A,B,C shortcodes, ends up with A,C,B with an empty D shortcode.

rogercoathup commented 8 years ago

We are seeing the problem if we have two TinyMCE wysiwyg content areas on the page, e.g. the main post editing area, and a custom meta box with a wysiwyg field.

If we update a shortcode in the custom meta box, the updated shortcode gets added to the main editing area, and doesn't get updated in the meta box wysiwyg field.

Note: it only happens when the shortcode is updated; it doesn't occur when one is first inserted

rogercoathup commented 8 years ago

Following up -- can no longer replicate the problem on a post page. Occurred once, but no more.

However, occurs all the time if I use 2 wysiwyg (TinyMCE) editors on a taxonomy term page. Shortcode that's updated in the lower editor, gets added to the top editor.

SOLUTION -- it appears to work correctly if I first click in the appropriate editor, then choose the edit option on the shortcode. It would seem that the editor needs to gain focus first.

zugrina commented 8 years ago

Same here, occurs all the time if I use multiple wysiwyg (TinyMCE) editors

goldenapples commented 8 years ago

@rogercoathup @zugrina - I think the issue you're reporting is a different one than what this ticket was initially about, but I'm not clear enough on how to reproduce. How are you registering these editors?

One thing to note about using Shortcake on term pages is that it's not actually supported at all currently - see https://github.com/wp-shortcake/shortcake/issues/599 for a bit more context, but in short, there's capability check in there checking if the current user can edit the current post before rendering the preview at all, so it will fail on term edit pages or other contexts.

hieu-luu-niteco commented 8 years ago

I used plugin ACF to create a wysiwyg (TinyMCE) field. When I have more than 1 shortcode in my custom field, the updating cause error. Instead of updating the shortcode, it created a new shortcode.

zugrina commented 8 years ago

I can confirm that I have the same problem cc @hieu-luu-niteco

rogercoathup commented 8 years ago

@goldenapples I bypassed the check (as suggested in referenced thread), to allow it to function on taxonomy term pages

StaggerLeee commented 7 years ago

Dont know why but I have feeling those two are connected: https://github.com/wp-shortcake/shortcake/issues/645

Sorry if I am mistaken, still very beginner for Shortcake UI.

StaggerLeee commented 7 years ago

Yes, I am afraid it is the same nasty problem. But now even in Firefox.

When I added some text before shortcode it started to work right each time when updating. And ACF editor field is not populating anymore with updates (it makes one new shortcode for each update).

ghost commented 7 years ago

Experiencing similar problem. Two wysiwyg custom fields, if the field with the shortcode doesn't have focus, the shortcode gets added to the other when editing. This still works on 4.5 though, the issue seems to only be present after upgrading to 4.6.

StaggerLeee commented 7 years ago

Does anyone have/know some handy snippet adding default content in editor ? It can be solved by adding horizontal line at the top of editor. Can be removed from frontend with one line of CSS code, not so big deal.

I tried "default_content" filter and it works but in some strange and unpredictable way. Sometimes it deletes all own content in editor.

I mean it is not solultion but would give developers peace of mind when they are fixing it. No need for stress.

StaggerLeee commented 7 years ago

Yes @MarcelLouwrens you are right. You noticed it right. It is about mouse focus. Focus only on shortcode is not enough. If any place inside editor is clicked first you can edit shortcode well.

tung-nt-niteco commented 7 years ago

Hi, did we have the fix for this "focus" problem yet?

rommeled commented 7 years ago

We use the plugin on more than 30 websites and can confirm there is still the issue with "editing" shortcodes in chrome would insert the shortcode in another edtitor on the admin post edit-screen. We noticed that it will always take the last of the editors available.

tung-nt-niteco commented 7 years ago

@rommeled Yes, it will always take the editor which has the focus. I am desperately looking forward to the fix as well.

rommeled commented 7 years ago

@tung-nt-niteco Here there was no foucs in the editor area which was inserted into. Actually we don't touch anything else than the specific shortcode in the first, of the three, editor areas and still it inserts into the last editor area available on the page.

tung-nt-niteco commented 7 years ago

Ah ok, that makes sense. Yes, if there is no focus, it will take the last editor.

StaggerLeee commented 7 years ago

I dont have this problem anymore, dont know why. Updated ACF to latest 5.5 v. Dont know if it has something to do with problem.

tung-nt-niteco commented 7 years ago

Tried to upgrade ACF pro to 5.5.1 and 5.5.9, but it did not solve the problem. If anyone even knew the solution, or if the shortcake author fixed it, please let us know.

goldenapples commented 7 years ago

Is anyone available to dig into this problem? I don't have an ACF Pro license to test it out.

tung-nt-niteco commented 7 years ago

I think the problem is not because of ACF Pro, the key to fixing this is "multiple" (as you switch between the HTML editors without paying attention to the mouse focus, you will get the problem). So the idea is to fix the ShortCake plugin instead of trying the external components. The suggestion is to include also parent-id (the id of the HTML editor) into the ShortCake elements and carry that parent-id to the ShortCake popup, instead of detecting the parent id on closing the popup. If so, we need the author of ShortCake plugin to fix it :)

tung-nt-niteco commented 7 years ago

Good news, the problem seems to be fixed. FYI, I am having: