Opened 4 years ago

Last modified 4 years ago

#1612 assigned defect

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

Reported by: ejucovy Owned by: ejucovy
Priority: normal Milestone:
Component: Xinha Core Version: trunk
Severity: major Keywords:
Cc:

Description

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.

Change History (3)

comment:1 Changed 4 years ago by ejucovy

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];
+    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
Last edited 4 years ago by ejucovy (previous) (diff)

comment:2 Changed 4 years ago by ejucovy

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];
 };
 
+/** 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];
+};
+
 /** 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

comment:3 Changed 4 years ago by ejucovy

  • Owner changed from gogo to ejucovy
  • Status changed from new to assigned
Note: See TracTickets for help on using tickets.