Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9427 closed defect (fixed)

[PATCH][CCLA] insertHorizontalLine is broken on IE.

Reported by: Jared Jurkiewicz Owned by: liucougar
Priority: high Milestone: 1.4
Component: Editor Version: 1.3.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Adam Peller)

This comes from a co-worker: Dojo RTE supports right align layout. But in IE, if add a horizontal line into the paragraph, the right align is broken.

  1. Enter some text into the RTE, and click align right.
  2. Place the cursor in front of the text, and insert a horizontal line.
  3. Check the source code, notice that the <hr> tag is inserted into the <p> tag.
  4. The browser will report js error when use the html code to render.
  5. The posted content will lost right alignment.

I have a simple fix for this, to special case the inserthorizontalrule action in RichText?, similar to how inserthtml is over-ridden.

The co-worker verified this does fix the problem and my testing of it shows it seems to fix it pretty well. Patch forthcoming.

Attachments (1)

editor_hr.patch (1.6 KB) - added by Jared Jurkiewicz 10 years ago.
Better version + update to UT for testing purposes.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 10 years ago by Adam Peller

Description: modified (diff)

I suggest we try to combine this logic with the inserthtml section.

comment:2 Changed 10 years ago by Jared Jurkiewicz

Summary: insertHorizontalLine is broken on IE.[PATCH][CCLA] insertHorizontalLine is broken on IE.

There are differences to the inserthtml section. The main one is you don't want to do a select after inserting the <hr>.

I avoided trying to combine them for fear of screwing up inserthtml

comment:3 Changed 10 years ago by Jared Jurkiewicz

This also works (and combines them)

var ihSelect = true; if(command === "inserthorizontalrule" && dojo.isIE){

IE doesn't do this right natively, so we have to use inserthtml logic with selection at the end disabled. command = "inserthtml"; argument = "<hr>"; ihSelect = false;

} if(command == "inserthtml"){

TODO: we shall probably call _preDomFilterContent here as well argument = this._preFilterContent(argument); returnValue = true; if(dojo.isIE){

var insertRange = this.document.selection.createRange(); if(this.document.selection.type.toUpperCase()=='CONTROL'){

var n=insertRange.item(0); while(insertRange.length){

insertRange.remove(insertRange.item(0));

} n.outerHTML=argument;

}else{

insertRange.pasteHTML(argument);

} if(ihSelect){

insertRange.select();

} insertRange.collapse(true);

}else if(dojo.isMoz && !argument.length){

mozilla can not inserthtml an empty html to delete current selection so we delete the selection instead in this case this._sCall("remove"); FIXME

}else{

returnValue = this.document.execCommand(command, false, argument);

}

}else if(

comment:4 Changed 10 years ago by Jared Jurkiewicz

Or even simpler (and why this didn't work the first time I tried it, I dunno):

if(command === "inserthorizontalrule" && dojo.isIE){

IE doesn't do this right natively, so we have to use inserthtml logic with selection at the end disabled. command = "inserthtml"; argument = "<hr>";

}

...

Allowing the select isn't causing issues. Could swear I saw problems the first time I allowed it. Gah. Oh well. This is simpler and I'm not seeing any problems from it.

Changed 10 years ago by Jared Jurkiewicz

Attachment: editor_hr.patch added

Better version + update to UT for testing purposes.

comment:5 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

(In [18121]) committing in fix for <hr>. \!strict fixes #9427

comment:6 Changed 10 years ago by Adam Peller

Milestone: tbd1.4
Note: See TracTickets for help on using tickets.