Closed Bug 857487 Opened 9 years ago Closed 9 years ago

Deleting a table row horked

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla23
Tracking Status
firefox20 --- affected
firefox21 + fixed
firefox22 + fixed
firefox23 + verified

People

(Reporter: glazou, Assigned: glazou)

References

Details

(Keywords: regression)

Attachments

(1 file)

Removing a table row through inline table editing is horked. This is
a regression.

1. launch a recent Nightly
2. open the Midas demo at http://www-archive.mozilla.org/editor/midasdemo/
3. click on the "View HTML Source" checkbox to switch to source view
4. paste the following markup:

   <table border="1" width="100%"><tbody><tr><td><br></td><td><br></td><td><br></td></tr><tr><td><br></td><td><br></td><td><br></td></tr><tr><td><br></td><td><br></td><td><br></td></tr></tbody></table><br>

5. switch back to wysiwyg view
6. place the caret inside a table cell
7. click on the circular button that appeared in the LEFT edge of the cell

Expected result: row is deleted
Actual result:   no action at all
After investigation, this is a regression introduced by fix for bug 781409. The guilty code is this one in nsHTMLEditor::GetCellDataAt:

  nsTableCellFrame* cellFrame =
    tableFrame->GetCellFrameAt(aRowIndex, aColIndex);
  if (!cellFrame)
    return NS_ERROR_FAILURE;

This creates the current bug since nsHTMLEditor::DeleteRow can call nsHTMLEditor::GetCellDataAt with an out-of-bound row and col index without expecting a fail; in particular see comment in line 1321 of nsTableEditor.cpp:

  // We don't fail if we don't find a cell, so this must be real bad
cc:ing :tbsaunde who handled bug 781409, cause of this regression.
Ok, this also affects all table-row deletions in the editor.
Tweaking bug's title accordingly.
Firefox inline editing, Thunderbird and BlueGriffon all affected.
Summary: Inline table editing partially horked → Deleting a table row horked
Assignee: nobody → daniel
Status: NEW → ASSIGNED
Attachment #732791 - Flags: review?(ehsan)
Comment on attachment 732791 [details] [diff] [review]
trivial fix + test

Review of attachment 732791 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #732791 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d428a8ddbdce

Daniel, can you please nominate this for Aurora and Beta approval as well?  Thanks!
Comment on attachment 732791 [details] [diff] [review]
trivial fix + test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 857487
User impact if declined: table row deletion in inline editors (Wikipedia, CKEditor, ...), Seamonkey, BlueGriffon non-functional.
Testing completed (on m-c, etc.): tested on m-c and in BlueGriffon's trunk
Risk to taking this patch (and alternatives if risky): extremely low, only the deepest code for table row deletion is touched
String or IDL/UUID changes made by this patch: none
Attachment #732791 - Flags: approval-mozilla-beta?
Attachment #732791 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d428a8ddbdce
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment on attachment 732791 [details] [diff] [review]
trivial fix + test

Low risk fix for a recent regression.Approving for uplift.

Thanks for the additional tests here :)
Attachment #732791 - Flags: approval-mozilla-beta?
Attachment #732791 - Flags: approval-mozilla-beta+
Attachment #732791 - Flags: approval-mozilla-aurora?
Attachment #732791 - Flags: approval-mozilla-aurora+
Verified as fixed on Firefox 23 Nightly(Build ID: 20130408030928) 
	
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:23.0) Gecko/20130408 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20130408 Firefox/23.0
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130408 Firefox/23.0
Duplicate of this bug: 862297
Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130421 Firefox/23.0   
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:23.0) Gecko/20130421 Firefox/23.0			
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0  
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:23.0) Gecko/20100101 Firefox/23.0

Verified as fixed on Firefox 21 (Build ID:20130416200523) and on Firefox latest nightly (Build ID:20130421031002).
Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0  
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0

Verified as fixed on Firefox 21 (Build ID:20130425162858).
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:23.0) Gecko/20130428 Firefox/23.0
Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20130428 Firefox/23.0

Verified as fixed on Windows 7 x64 and Ubuntu 12.04 x86_64 with latest Nightly 23.0a1 (Build ID: 20130429030926)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:23.0) gecko/20130503 Firefox/23.0

Verified on Mac - latest Nightly - in addition to Maria's verification on Ubuntu and Windows.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 858309
You need to log in before you can comment on or make changes to this bug.