423514 - "ASSERTION: cellmap is lying" with rowspan, contenteditable, rtl

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jesse Ruderman]

Attachment: testcase

The testcase triggers this assertion a few times when it is loaded, and then again every time the caret blinks.

###!!! ASSERTION: cellmap is lying: 'Error', file /Users/jruderman/trunk/mozilla/layout/tables/nsTableRowGroupFrame.cpp, line 1675

This assertion was added by the patch in bug 388700.

Comment 1

[Bernd]

once cvs is up again I will attach a patch that will change the code 

to
  *aNumFramesOnLine = cellMap->GetNumCellsOriginatingInRow(aLineNumber);
  if (*aNumFramesOnLine == 0) {
    return NS_OK;
  }
  PRInt32 colCount = table->GetColCount(); <<< we need this
  for (PRInt32 i = 0; i < colCount; i++) {

Otherwise we have in the first cell a rowspan from above and will not lookup the second cell where the originating cell that made aNumFramesOnLine to be 1 will be found.

Comment 2

[Bernd]

Attachment: patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 310539 [details]
patch

Indeed.  r+sr=bzbarsky

Comment 4

[Bernd]

Comment on attachment 310539 [details]
patch

this change is low risk and corrects a obviously wrong code.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 310539 [details]
patch

a1.9=beltzner

Comment 6

[Bernd]

fix checked in

Comment 7

[Jesse Ruderman]

I checked in the testcase (with a change to make it wait 1200ms so the caret can blink) as a crashtest.

Comment 8

[Boris Zbarsky [:bzbarsky]]

If this test depends on the caret blink time, can we add a comment where that blink time  is defined saying so?

Comment 9

[Jesse Ruderman]

Caret blink time seems to be OS-dependent based on a skim of http://lxr.mozilla.org/mozilla/search?string=caretblink.  I pulled "1200ms" out of thin air.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Ah, hmm.  I wonder whether we can write some reftests that use != to make sure the caret is in fact blinking or something....  Seems difficult. :(

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You can't currently, because canvas.drawWindow doesn't ever draw the caret.

Comment 12

[Martijn Wargers (dead)]

(In reply to comment #11)
> You can't currently, because canvas.drawWindow doesn't ever draw the caret.

Well, that became worksforme, see bug 394763.

Comment 13

[Masahiro YAMADA]

bug 426708 seems to be related to this bug

Comment 14

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).