Opened 6 years ago

Closed 5 years ago

#16871 closed defect (patchwelcome)

dijit/_editor/plugins/LinkDialog createLink action uses inserthtml command which breaks up containing elements

Reported by: Nick Fenwick Owned by:
Priority: undecided Milestone: tbd
Component: Editor Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

The nightly tests pages don't include an example of LinkDialog? in action, but the docs page does. http://dojotoolkit.org/reference-guide/1.8/dijit/Editor.html#additional-editor-plugins

Run the example under Plugins with the sentence "This example starts from scratch, thus removing some items from the toolbar (as compared to the default), like underline, and adding other features, namely the LinkDialog:" above it.

  1. Note that the initial text contains a simple <p> containing text.
  2. Select some words in that <p> and hit Bold. This creates <p>Some text <b>your selected text</b> some more text</p>.
  3. Select a word in that bold section and click the Create Link toolbar, and type a simple "test" URL. Hit 'Set'.

-- expected results --

The <a> that is put in the content should be 'within' the <b> tag and thus its contents retain the bold styling.

-- actual results --

The resultant HTML (in Firefox at least) is:

<p>This <b>instance is </b><a href="test" _djrealurl="test" target="_self">created</a><b> with customized</b> toolbar/ plugins</p>

Thus the anchor that is inserted does not retain the styling of the original text. In our application it is contained within <font> tags and breaks usability.

-- suggested fix --

Instead of using the contentEditable 'inserthtml' command to insert an anchor tag as a string block like "<a href='foo' _djrealurl='test' target='_self'>created</a>", we should use the command 'createlink'. See e.g. https://developer.mozilla.org/en/docs/Rich-Text_Editing_in_Mozilla

I have extended LinkDialog? and replaced its _setValue function to use this code:

// make sure values are properly escaped, etc.
args = this._checkValues(args);
/** COMMENTED OUT old code **/
//this.editor.execCommand('inserthtml',
	//string.substitute(this.htmlTemplate, args));
/** NEW CODE below **/
this.editor.execCommand('createlink', args.urlInput);
// Now mangle the missing pieces from htmlTemplate
var sel = rangeapi.getSelection(this.editor.window);
var range = sel.getRangeAt(0);
var a = range.endContainer,
	insertedAnchor = a.parentNode;
domAttr.set(insertedAnchor, {
	'_dlrealurl': args.urlInput,
	target: args.targetSelect
});

Thus we no longer use the htmlTemplate and its string substitution to build the <a> tag that is being created, we let the browser do the creation.

The drawback is that one must identify the <a> node that has been inserted and put our _djrealurl and target attributes on it. I'm not very familar with the range api but it seems this method of obtaining the node at the current selection is reliable, I borrowed it from elsewhere in the plugin code. I've tested it in the latest Chrome, latest Firefox, and IE8.

Before I look much more at this, would this be considered a bug? Is a patch like this, to use the 'createlink' command, acceptable?

Change History (2)

comment:1 Changed 6 years ago by bill

Yes, definitely seems like a bug, and this seems like a good solution although I'm unclear if it's portable or FF only? Anyway, if you can supply a patch file including an automated test case (i.e. adding a test case to Editor_a11y.html, or Editor_mouse.html, etc.), I'd be happy to look at it.

comment:2 Changed 5 years ago by bill

Resolution: patchwelcome
Status: newclosed

Unlikely this will be fixed unless someone wants to supply a patch (including a test case).

Note: See TracTickets for help on using tickets.