Pressing enter at end of an li:last-child with a child node lands you in the list itself (Trac #1612)

[sleemanj]

To reproduce:

1. Open a Xinha editor with the following contents: <ul><li><span style="color: green">Lorem ipsum</span></li></ul>
2. Move the cursor to just after the "Lorem ipsum" text
3. Press enter

Expected behavior: a new, empty <li> should be created on the next line. The cursor should be inside the new list item.

Actual behavior: the cursor is on a new line, outside of the existing <li>, but no new <li> has been created. Instead a <p> has been created at the end of the <ul>, and the user is now able to enter text in the ul > p without any containing list item. As a result, the text is indented (since it's inside a list) but with no bullet (since it's not inside a list item) and the HTML is malformed.

Reproduced using Chrome 31.0.1650.63 and Firefox 26.0, against xinha trunk@1327.

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

[sleemanj]

• ejucovy changed _comment0 from:

The following patch fixes the problem:

#!diff
Index: XinhaCore.js
===================================================================
--- XinhaCore.js  (revision 1327)
+++ XinhaCore.js  (working copy)
@@ -5120,7 +5120,10 @@
   {
     types = [types];
   }
+  return this._getFirstAncestorForNodeAndWhy(prnt, types);
+};

+Xinha.prototype._getFirstAncestorForNodeAndWhy = function(prnt, types) {
   while ( prnt )
   {
     if ( prnt.nodeType == 1 )
Index: modules/Gecko/paraHandlerBest.js
===================================================================
--- modules/Gecko/paraHandlerBest.js  (revision 1327)
+++ modules/Gecko/paraHandlerBest.js  (working copy)
@@ -749,8 +749,11 @@
   {
     // neither we nor our parent are a list item. this is not a normal
     // li case.
-    
-    return false;
+    var listNode = editor._getFirstAncestorForNodeAndWhy(node, ["li"])[0](../commit/0);
+    if ( typeof listNode == 'undefined' )
+    {
+      return false;
+    }
   }

   // at this point we have a listNode. Is it the first list item?
@@ -904,4 +907,4 @@

 };   // end of handleEnter()

-// END
\ No newline at end of file
+// END

to:

1388935701754199

• ejucovy commented:

The following patch fixes the problem:

Index: XinhaCore.js
===================================================================
--- XinhaCore.js  (revision 1327)
+++ XinhaCore.js  (working copy)
@@ -5120,7 +5120,10 @@
   {
     types = [types];
   }
+  return this._getFirstAncestorForNodeAndWhy(prnt, types);
+};

+Xinha.prototype._getFirstAncestorForNodeAndWhy = function(prnt, types) {
   while ( prnt )
   {
     if ( prnt.nodeType == 1 )
Index: modules/Gecko/paraHandlerBest.js
===================================================================
--- modules/Gecko/paraHandlerBest.js  (revision 1327)
+++ modules/Gecko/paraHandlerBest.js  (working copy)
@@ -749,8 +749,11 @@
   {
     // neither we nor our parent are a list item. this is not a normal
     // li case.
-    
-    return false;
+    var listNode = editor._getFirstAncestorForNodeAndWhy(node, ["li"])[0](../commit/0);
+    if ( typeof listNode == 'undefined' )
+    {
+      return false;
+    }
   }

   // at this point we have a listNode. Is it the first list item?
@@ -904,4 +907,4 @@

 };   // end of handleEnter()

-// END
\ No newline at end of file
+// END

[sleemanj]

ejucovy commented:

Cleaned up the patch a little:

Index: XinhaCore.js
===================================================================
--- XinhaCore.js  (revision 1327)
+++ XinhaCore.js  (working copy)
@@ -5094,6 +5094,16 @@
   return this._getFirstAncestorAndWhy(sel, types)[0](../commit/0);
 };

+/** Traverses the DOM upwards and returns the first element that is of one of the specified types
+ *  @param {DomNode} node The DOM node whose ancestors we want to traverse.
+ *  @param {Array|String} types Array of matching criteria.  Each criteria is either a string containing the tag name, or a callback used to select the element.
+ *  @returns {DomNode|null} 
+ */
+Xinha.prototype._getFirstAncestorForNode = function(node, types)
+{
+  return this._getFirstAncestorForNodeAndWhy(node, types)[0](../commit/0);
+};
+
 /** Traverses the DOM upwards and returns the first element that is one of the specified types,
  *  and which (of the specified types) the found element successfully matched.
  *  @param {Selection} sel  Selection object as returned by getSelection
@@ -5120,7 +5130,16 @@
   {
     types = [types];
   }
+  return this._getFirstAncestorForNodeAndWhy(prnt, types);
+};

+/** Traverses the DOM upwards and returns the first element that is one of the specified types,
+ *  and which (of the specified types) the found element successfully matched.
+ *  @param {DomNode} prnt A DOM node whose ancestors we want to traverse.
+ *  @param {Array|String} types Array of matching criteria.  Each criteria is either a string containing the tag name, or a callback used to select the element.
+ *  @returns {Array} The array will look like [{DomNode|null}, {Integer|null}] -- that is, it always contains two elements.  The first element is the element that matched, or null if no match was found. The second is the numerical index that can be used to identify which element of the "types" was responsible for the match.  It will be null if no match was found.  It will also be null if the "types" argument was omitted. 
+ */
+Xinha.prototype._getFirstAncestorForNodeAndWhy = function(prnt, types) {
   while ( prnt )
   {
     if ( prnt.nodeType == 1 )
Index: modules/Gecko/paraHandlerBest.js
===================================================================
--- modules/Gecko/paraHandlerBest.js  (revision 1327)
+++ modules/Gecko/paraHandlerBest.js  (working copy)
@@ -747,10 +747,14 @@
   }
   else
   {
-    // neither we nor our parent are a list item. this is not a normal
-    // li case.
-    
-    return false;
+    // neither we nor our parent are a list item. now let's check
+    // if we have any direct ancestor that is a list item.
+    var listNode = editor._getFirstAncestorForNode(node);
+    if ( typeof listNode == 'undefined' )
+    {
+      // we are not directly descended from any list item.
+      return false;
+    }
   }

   // at this point we have a listNode. Is it the first list item?
@@ -904,4 +908,4 @@

 };   // end of handleEnter()

-// END
\ No newline at end of file
+// END

[sleemanj]

• ejucovy changed owner from gogo to ejucovy
• ejucovy changed status from new to assigned

[sleemanj]

• @sleemanj commented:

1383 Implements this

@NOTE Reporter - this may be the first notification you will have seen from activity on Xinha in years. If you currently use Xinha you might want to check the Timeline at trac.xinha.org , to borrow from Twain, reports of it's death have been greatly exaggerated.

• @sleemanj changed resolution to fixed
• @sleemanj changed status from assigned to closed