sleemanj / xinha

WYSIWYG HTML Editor Component (turns <textarea> into HTML editors)
http://trac.xinha.org/
Other
13 stars 2 forks source link

Changing heading type in a list creates nested headings (Trac #1615) #1615

Closed sleemanj closed 3 years ago

sleemanj commented 10 years ago

To reproduce:

  1. Create a bulleted list and enter some text in the first list item. (HTML: <ul><li>Foo</li></ul>
  2. Put the cursor anywhere inside the list item
  3. Press ctrl+1 or use the Headings dropdown to make the text an <h1>. (HTML: <h1><ul><li>Foo</li></ul></h1>
  4. Press ctrl+2 to make the text an <h2> instead.

The HTML will now be: <h1><h2><ul><li>Foo</li></ul></h2></h1>

The expected behavior would be to replace the H1 tag with an H2 tag.

That said, I think there's also a bug earlier in these steps: the <h1> should be wrapped around the contents of the current <li>, rather than wrapping the entire <ul>. This is (slightly) more likely to be the user's intention, and http://stackoverflow.com/a/5014827/689985 suggests that the latter is valid HTML while the former is invalid.

Reported by ejucovy, migrated from http://trac.xinha.org/ticket/1615

sleemanj commented 10 years ago

ejucovy commented:

This bug exists using Chrome 31.0.1650.63.

It does not exist using Firefox 26.0 -- in that browser, the behavior is exactly as it should be, per the last paragraph of the ticket. (Resulting HTML is <ul><li><h2>Foo</h2></li></ul> in Firefox.)

sleemanj commented 10 years ago

This must be a chrome bug, confirm it still exists in Chrome 65, but not in IE 11 or FF 58

The format (heading) is done by the browser's execCommand. Perhaps your ContentEditable plugin could help, but that seems like a lot of code to solve this problem.

In Xinha.prototype._comboSelected

    case "formatblock":
      // Mozilla inserts an empty tag (<>) if no parameter is passed  
      if ( !value )
      {
          this.updateToolbar();
          break;
      }
      if( !Xinha.is_gecko || value !== 'blockquote' )
      {
        value = "<" + value + ">";
      }
      this.execCommand(txt, false, value);
    break;

I think that given the rarity of wanting to put a header in a list item, wontfix.