new rows inserted in a table do not inherit the class attribute from the previous row

NEW
Assigned to

Status

()

defect
16 years ago
Last year

People

(Reporter: pascalc, Assigned: laurent, NeedInfo)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

16 years ago
build 2003043008 winXP

1 edit a table in composer with some cells having a class attribute
(like <tr><td></td><td class="foo"></td></tr>)
2 insert a new row from context menu (table insert/row blelow)

expected result : a new row with cells inheriting the class attribute from the
previous line
actual result : a new row without class attributes
Very interesting, indeed.
QA Contact: bugzilla → editor
Assignee: mozeditor → nobody
Assignee

Comment 2

11 years ago
I take it.
Assignee: nobody → laurent
Assignee

Comment 3

11 years ago
This patch add a new method on nsITableEditor

   void insertTableRowExt(in long  aNumber, in boolean aAfter,
                          in boolean aSameStyleAndType,
                          in boolean aSameColSpan,
                          in boolean aPreserveRowSpan);

So we can says if the new row will add same style, same colspan on its cell, and if it preserves rowspan.

And this method is called when we hit the tab key in the editor, instead of insertTableRow.
Attachment #353688 - Flags: review?(daniel)
Attachment #353688 - Flags: review?(daniel) → review+
Comment on attachment 353688 [details] [diff] [review]
Patch to add a new method insertTableRowExt


>+  /**
>+    * Insert relative to the selected cell or the 
>+    *  cell enclosing the selection anchor
>+    * The selection is collapsed and is left in the new cell
>+    *  at the same row,col location as the original anchor cell
>+    *
>+    * @param aNumber    Number of items to insert
>+    * @param aAfter     If TRUE, insert after the current row,
>+    *                     else insert before current row
>+    * @param aSameStyleAndType if TRUE, it generates same cell type (td or th)
>+    *                           and applies same style attributes (style, class, valign...)
>+    * @param aSameColSpan if TRUE, colspan are preserved
>+    * @param aPreserveRowSpan if TRUE, rowspan are preserved
>+    */
>+  void insertTableRowExt(in long  aNumber, in boolean aAfter,
>+                         in boolean aSameStyleAndType,
>+                         in boolean aSameColSpan,
>+                         in boolean aPreserveRowSpan);

Why not aPreserveStyleAndType, aPreserveColSpan and aPreserveRowSpan ?

>@@ -1414,17 +1414,17 @@ NS_IMETHODIMP nsHTMLEditor::TabInTable(P
>       return NS_OK;
>     }
>   } while (!iter->IsDone());
>   
>   if (!(*outHandled) && !inIsShift)
>   {
>     // if we havent handled it yet then we must have run off the end of
>     // the table.  Insert a new row.
>-    res = InsertTableRow(1, PR_TRUE);
>+    res = InsertTableRowExt(1, PR_TRUE, PR_TRUE, PR_TRUE, PR_TRUE);

Right, this is the only place where we want this thange for the time being.
Clients of this code like Thunderbird or Composer/BlueGriffon will decide
by themselves if they want to change their "Insert Row above/below" calls.

>+
>+  // The list of cell to "duplicate".
>+  nsVoidArray cellList;

"The list of cells"

>+            cellsInRow ++;

cellsInRow++; please with no whitespace


>+        if (!aPreserveRowSpan) {
>+          SetRowSpan(curCell, actualRowSpan);
>+        }
>+        else
>+          checkColSpan = PR_FALSE;

Please invert the two cases and base the test on aPreserveRowSpan.

>+          cellsInRow ++;

cellInRow++


>+          for (i=0; i < actualColSpan; i++) {
>+            cellList.AppendElement((void*)curCell.get());
>+          }

i = 0;

>+            cellPtr->GetAttribute(NS_LITERAL_STRING("valign"), attr);
>+            if (!attr.IsEmpty())
>+              newCell->SetAttribute(NS_LITERAL_STRING("valign"), attr);
>+            else
>+              // we remove the valign attribute added by CreateElementWithDefaults
>+              newCell->RemoveAttribute(NS_LITERAL_STRING("valign"));

Same as above.

>+            cellPtr->GetAttribute(NS_LITERAL_STRING("style"), attr);
>+            if (!attr.IsEmpty())
>+              newCell->SetAttribute(NS_LITERAL_STRING("style"), attr);
>+            else
>+              // we remove the style attribute added by CreateElementWithDefaults
>+              newCell->RemoveAttribute(NS_LITERAL_STRING("style"));

Same as above.

r=me provided the requested changes are made
Assignee

Comment 5

11 years ago
it adresses glazou's comments, and adds an improvement : attributes of the duplicated tr element were not copied on the new tr element. This is why I ask a new review :-)
Attachment #353688 - Attachment is obsolete: true
Attachment #356522 - Flags: review?(daniel)
Reporter

Comment 6

Last year
NI myself to remind me of checking if this bug is still valid.
Flags: needinfo?(pascalc)
You need to log in before you can comment on or make changes to this bug.