Opened 9 years ago

Closed 9 years ago

#10801 closed defect (fixed)

Editor : placeCursorAtStart problem on FF if Editor content starts with <br/>

Reported by: jfcunat Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.4.2
Component: Editor Version: 1.4.0
Keywords: Cc: Jared Jurkiewicz; gmouricou.ext@…
Blocked By: Blocking:

Description

On Firefox 3.5, when using placeCursorAtStart method on an Ediot that contains content beginning with <br/>, you are not able to type anymore. Cursor is well blinking at the start of the content but you are not able to type anything.

This seems to come from the specific code done for FF in placeCursorAtStart method in RichText?.js . Seems that the behaviour should the same for text node (nodeType=1) than for nodeType 3 element with no childNode.

Test case provided (just click on cursor at start button and try to type in Editor)

Attachments (2)

test_Editor_insertAtStart.html (1.2 KB) - added by jfcunat 9 years ago.
childlessTag.patch (939 bytes) - added by Jared Jurkiewicz 9 years ago.
Patch for problem

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by jfcunat

Changed 9 years ago by Jared Jurkiewicz

Attachment: childlessTag.patch added

Patch for problem

comment:1 Changed 9 years ago by Jared Jurkiewicz

The problem, as best I can determine, occurs because certain tags, such as <br> are childless tags, meaning they cannot have text or anything else within them. The logic as it stood would position the cursor inside any tag, childless or not. So what would happen would it would get stuck as you can't type insde a childless tag.

Simple fix (as done in patch), is to determine if the node can have children or not, and position cursor accordingly (before if it cannot, inside if it can).

comment:2 Changed 9 years ago by Jared Jurkiewicz

(In [21445]) Patching in a quick fix for this by checking that the first tag is not a childless tag and if it is, position before it. Should revisit logic in 1.5 as I do not think the browser branching is required here. \!strict refs #10801

comment:3 Changed 9 years ago by Jared Jurkiewicz

Comitting in a quick fix for 1.4.2 (it's not a regression, though, same issue on 1.3). Will revisit and improve for 1.5

comment:4 Changed 9 years ago by Jared Jurkiewicz

jfcunat,

Can you test this on Dojo 1.4.2 RC1, please? It will be cut very soon.

comment:5 Changed 9 years ago by Jared Jurkiewicz

Owner: set to Jared Jurkiewicz

Added in trunk: [21455]

comment:6 in reply to:  4 Changed 9 years ago by jfcunat

Replying to jaredj:

jfcunat,

Can you test this on Dojo 1.4.2 RC1, please? It will be cut very soon.

This is Ok for us, but is it for a performance reason that you do not use childNodes.length as a test ? It it is the case some other empty tags could be added in your test : hr, link?,

comment:7 Changed 9 years ago by Jared Jurkiewicz

It is not an issue with the tag being empty, it is an issue with the tag not being allowed to have children. childNodes.length would be a bad test. For example, a div with no children is a valid node to select into.

comment:8 Changed 9 years ago by Adam Peller

Resolution: fixed
Status: newclosed

1.4.2 is out. closing ticket. If more work needs to be done, you can open another ticket or just ref this one, I guess.

comment:9 Changed 9 years ago by jfcunat

Resolution: fixed
Status: closedreopened

Could you just add hr tag in the list of childless tag ?

comment:10 Changed 9 years ago by Jared Jurkiewicz

Added hr, link + test in [21564]

comment:11 Changed 9 years ago by Jared Jurkiewicz

Resolution: fixed
Status: reopenedclosed

(In [21565]) Adding in hr and link select-before to 1.4 branch. \!strict fixes #10801

Note: See TracTickets for help on using tickets.