#42059 closed defect (bug) (fixed)
Editor: preview for embedding from URL breaks on switching Text to Visual when selected
Reported by: | azaozz | Owned by: | biskobe |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Editor | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
To test:
- Paste embeddable URL like https://www.youtube.com/watch?v=bpoIYVggmVA in the Visual editor, wpview kicks in and shows a preview.
- Switch to Text.
- Select (part of) the URL or place the caret in it.
- Switch to Visual.
At this point the preview is not shown, the URL remains as string. If the selection is outside the URL in Text mode, the preview is shown properly.
Attachments (3)
Change History (13)
#1
@
7 years ago
- Keywords needs-patch added
- Owner set to biskobe
- Status changed from new to assigned
#3
@
7 years ago
Instead of using span(s) for the selection marker, perhaps better to use a zero-width element like <wbr data-wp-selection="start|end" />. These will be quite easier to remove from HTML strings. We can also style them in CSS, no need for inline styles.
Great suggestion. Going to implement that, together with the other issues you mentioned in the ticket.
Keeping selected text is ok, but not essential imho. It's rare to switch editors while something is selected. The important part is to keep the position of the caret and to scroll to it after switching. If there is a selection, consider collapsing it to the beginning and using only one <wbr> for a marker.
I kind of disagree with that. It's not common, because we're used to not having the selection transferred between the Editor modes. Keeping the whole selection creates a better experience as the user can see what they have selected when going to the other mode.
Collapsing the selection to either end will only give us the cursor, which is hard to find in either mode. Imagine having a full screen of text and you have to find a tiny cursor that blinks somewhere in it.
That way all previews and "dynamic blocks" will work properly no matter where they come from.
So far the only exception I could see to the general Dynamic Blocks / Live Preview is the URL embed issue you mentioned in the ticket, as it's not a defined shortcode with a Live Preview functionality. Do you see any possibility of having dynamic blocks that do not follow the general rule for Live Preview? If there is such case, it would be awesome to see it and test with it, since even having a single marker can break them if it's not positioned properly.
#5
@
7 years ago
Things the patch fixes:
- Pasted URLs are now treated as blocks. If the cursor is inside one of them, the cursor will be pushed to the start/end of the URL, like HTML tags and shortcodes are treated.
- Refactored underscore.js usages to use native JS
- Marking the selection markers as
data-mce-bogus
as early as possible, so they are removed from the content if something happens down the line in the script.
#6
@
7 years ago
@azaozz I decided to forego the migration to <wbr data-wp-selection="start|end" />
, since it introduced some interesting issues with detecting the selection positions and at least Chrome can't detect its position on the page, since it's rendered display: inline
by default.
I'll be happy to circle around later after finishing up issues that may arise from testing the selection functionality.
#7
@
7 years ago
42059.2.diff
adds some more improvements over the previous patch:
- A fix for a JS error that was reported in #40854 ( https://core.trac.wordpress.org/ticket/40854#comment:33 ).
- Introduced Editor settings/properties to allow/disallow selection keeping and scrolling to the selected position.
- They're false by default.
- A minor cleanup to remove a parameter that was passed, but wasn't required by the function.
Couple more ideas:
<wbr data-wp-selection="start|end" />
. These will be quite easier to remove from HTML strings. We can also style them in CSS, no need for inline styles.