Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Chrome inserts a span element with a "line-height" style after inserting a line break with Shift+Enter then deleting it #424

Open
NickMcL opened this issue Jul 30, 2013 · 2 comments

Comments

@NickMcL
Copy link
Contributor

NickMcL commented Jul 30, 2013

The following reproduction will cause a span element with a style of "line-height: 5px;" to be inserted inside a paragraph in Chrome 27:

  1. Go to the basic.html test page.
  2. Put the cursor after the end of the text in the paragraph.
  3. Press Shift+Enter to insert a line break inside the paragraph.
  4. Press Backspace to delete the line break just inserted.
  5. Press Enter to insert a new paragraph container below the current one.
  6. Type some text. The text will be inside a span element with a style of "line-height: 5px;" within the new paragraph container.

If you continue to hit Enter to create new paragraph containers after this, it will insert the same span element in each new paragraph.

This span element should not be there, so it should be removed.

@winhamwr
Copy link
Member

I've been trying to think through a way of dealing with this and the entire class of problems around spans that get inserted with styling that we want to ignore. The same kind of weird nonsense happens even if you try to copy/paste text within the editor from one section to another section, if either section has different font sizes as a result of being different element types.

It seems like the best course of action is to have both the parser and the editor content do correction, but the difficult design goals are to:

  • Define a clear relationship with how they work together
  • Make them performant by only calling them when needed
  • Avoid implementing every change twice, once in the DOM and once in the parser

Parser does the heavy lifting

All of the detailed corrections should happen in the parser. Things like saying that if something has the class msoList it's actually a list that was pasted from Word, and we should strip that and change the formatting. See tinyMCE's paste plugin test suite as an example of the kind of things you need to deal with in Word.

We can start off by stripping style attributes from everything and by removing any spans that only had a style.

Then, we one at a time build up the cases with things like "If a span has class X and style Y, then it's actually a heading, so convert its surrounding paragraph to a heading and unwrap the span."

Handle likely-breaking small events with the normal fixBodyHtml process

I'm not sure exactly when the span with the line-height emerges in this case, but hopefully it happens right after the Enter keystroke or some such. If that's the case, we can just handle fixing some basic cases (like that span) on the keyUp of the POTENTIAL_BLOCK_ELEMENT_CREATION_KEYS.

Call out to the parser on big events

When we get a paste event, or evidently some kind of drop event as referenced in #391, then we should do a full pass through the update call which goes through the parser and then re-adds any necessary editor-only things. That will help us handle complicated cleanup actions like tables, lists, formatted content, etc.

Alternatives

It seems like most of the other editors have settled on using tricks to capture pasted content with an invisible div that they hackily attempt to move selection in to. Its not consistent and it doesn't work if the user uses a menu to paste instead of the keyboard shortcut (and I'm guessing most of our users use the menu). It also creates a lot of special-casing where you do different things with pasted content compared to normal content. I used to think this was the best way.

Now I'm of the opinion that we can take advantage of why WYMeditor is different to solve this. We're opinionated about only producing well-structured, semantic HTML and prioritizing that over giving the end user complete control. So let's crank that up to 11 and build the corrections that work both for HTML that already existed, along with changes that the user makes at edit time.

@mightyiam mightyiam removed this from the 1.0.0 milestone Jun 10, 2014
@mightyiam
Copy link
Contributor

I've removed the 1.0.0 milestone.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants