Opened 11 years ago
Closed 10 years ago
#29841 closed defect (bug) (fixed)
Don't replace non embeddable URLs with a loading placeholder
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | 4.0 |
Component: | TinyMCE | Keywords: | |
Focuses: | javascript | Cc: |
Description
Continuing from #29268.
This is visually annoying.
Also, the cursor is set back to the start of the url.
Attachments (9)
Change History (18)
#3
@
11 years ago
Oh, I thought that was about urls still embedding in the editor when oembed is disabled.
#4
@
11 years ago
I think we can keep the two tickets separate - the other for if you want to turn off oEmbed (although perhaps there's a better/different way to accomplish that?), and this for this issue that was noted pre-release but not addressed. Not sure what our options are - whitelisting in JS, a la wpautop? Something else?
#5
@
11 years ago
Nah, we just shouldn't use a loading placeholder. Only replace the URL when there's something to replace it with. We'll need to change the way wpviews load though.
#6
@
11 years ago
- Keywords has-patch needs-testing added
- Milestone changed from Future Release to 4.2
Initially I wanted to approach this differently by not setting a placeholder, but that wouldn't be ideal for shortcodes (ideally the user should never see the shortcode).
Instead it would be better check the url with the regexes of the providers. So this patch converts them for JS and adds an additional check before replacing with a loading placeholder. I'm not a regex expert, so this should definitely be checked/tested.
This patch will also set the cursor to the end of the view if the cursor is within the view so you can continue typing.
#7
@
11 years ago
Don't think we can reuse the same regex as in PHP. There are quite a few syntax differences between PCRE and JS. It's true we can keep the default regex compatible, but plugins can add there too. Trying to convert from PCRE to JS syntax is no fun.
Thinking we should go with the first option: having another arg on wp.mce.views.toViews()
to indicate that we don't want a loading placeholder while waiting for the response. This would be a good addition that may be useful in other contexts too.
We can get this working in couple of ways: keep a reference to the content of the <p> node in the TinyMCE plugin, fire an event when the server sends the embed code, check if the node content/text hasn't changed, then either add the whole wpView or leave it as-is if no match. Or we can match that node to a particular ajax request and replace on completion.
Duplicate of #29686?