Closed Bug 255381 Opened 20 years ago Closed 11 years ago

'rowIndex' property not accessible on tables built dynamically (but cellIndex works!)

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: nicholas, Unassigned)

Details

(Keywords: html5, testcase)

Attachments

(2 files)

Testcase
659 bytes, text/html
Details
Fix for the GetTable part
1011 bytes, patch
sicking
: review+
jst
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040805 Firefox/0.9.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040805 Firefox/0.9.3

The 'rowIndex' property is not accessible on tables built dynamically (but
cellIndex works!).  When trying to access this value from a table row created in
HTML, it works fine.  But when the row made through javascript and the DOM tree,
nothing happens and we get a error message on the javascript console:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004002 (NS_NOINTERFACE) [nsIDOMHTMLTableRowElement.rowIndex]"  nsresult:
"0x80004002 (NS_NOINTERFACE)"  location: "JS frame ::
http://192.168.1.100/GRAY/master2.html :: anonymous :: line 28"  data: no]

Reproducible: Always
Steps to Reproduce:
Here is sample code that will make a table with one row that, when clicked, will
reveal the rowIndex.  The javascript will then append another row onto the table
with the same onclick event.  When this is clicked, the rowIndex property
becomes inaccessible and does not display; instead, we get the following
javascript console error:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004002 (NS_NOINTERFACE) [nsIDOMHTMLTableRowElement.rowIndex]"  nsresult:
"0x80004002 (NS_NOINTERFACE)"  location: "JS frame ::
http://192.168.1.100/GRAY/master2.html :: anonymous :: line 28"  data: no]

*******************  TEST CODE ******************************8

<html>
<body>

<table border="1" id="t1">
	<tr onclick="alert(this.rowIndex);">
		<td>works here</td>
	</tr>
</table>

<script>
	var table = document.getElementById("t1");	// find table
	
	var row = document.createElement("tr");		// make row element
		
	var cell = document.createElement("td");	// make cell element

	var content = document.createTextNode("fails here"); // add text

	cell.appendChild(content);		// add text to cell

	row.appendChild(cell);			// add cell to row

	row.onclick = Function("alert(this.rowIndex)");
        // this row gets the same function as first row

	table.appendChild(row);			// add new row to table
</script>
</body>
</html>

Actual Results:  
Only row 0 seems to have a rowIndex attached to it.  Row 1 does not, it seems,
because it was created dynamically via the DOM.  Thus, we get an error in the
javascript console as follows:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004002 (NS_NOINTERFACE) [nsIDOMHTMLTableRowElement.rowIndex]"  nsresult:
"0x80004002 (NS_NOINTERFACE)"  location: "JS frame ::
http://192.168.1.100/GRAY/master2.html :: anonymous :: line 28"  data: no]

Expected Results:  
Both rows, when clicked, should alert the user with the respective rowIndex
value (0 and 1, in this case).  The cellIndex property DOES function properly on
dynamically created cells, it seems.  

I haven't yet discovered a work-around or a situation where this problem vanishes...
Attached file Testcase — 
The problem, of course, is that the new row is not being placed in the table
body, unlike the old one...  Please see the resulting page DOM in DOM
Inspector.
It's easy enough to fix this to not throw (by fixing
nsHTMLTableRowElement::GetTable). But then the rowindex will be -1, since the
rows collection only sees rows that are in table sections.

Note that the definition of rowIndex in
http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-6986576 does seem to
presuppose that all table rows are in table sections (though this is not true in
XHTML).
I wouldn't say that the spec says that .rows and .rowIndex only takes rows in
sections into account, I would actually claim the opposite, at least for .rows.
What it doesn't do though is say how rows outside of sections are positioned.

I would say that we should fix nsHTMLTableRowElement::GetTable (or kill it, it
seems to only have one caller), and then make .rows contain <tr>s that are
direct children of the <table>. IMHO these rows should show up in the position
where they are rendered.
Status: UNCONFIRMED → NEW
Ever confirmed: true
> IMHO these rows should show up in the position where they are rendered.

That would take some fun work, actually (pretty much have to completely
reimplement .rows).
Yep, i'm well aware of that, and it sucks, but it feels like the RightThing to do.

Though i'm not sure we would need a full reimplementation. We should still be
able to use the table-section elements implementation to index the rows that are
in sections. We would "just" need to build a list of direct-child-rows and
insert those rows at appropriate places as we step through the tbodies list.

We probably won't have to bother with performance for the case when both tbodies
and direct-child-rows exist since that should be very rare.
The "just" insertion would mean a lot of calls to CompareTreePosition or
something....

Perhaps what we should do is have a content list that has all trows/tbodies in
it, and walk that, calling down into the tbodies as needed (so instead of
GetTBodies, actually use GetTBodiesAndOrphanRows or something)?
Comment on attachment 155960 [details] [diff] [review]
Fix for the GetTable part

This part we should just get in.
Attachment #155960 - Flags: superreview?(jst)
Comment on attachment 155960 [details] [diff] [review]
Fix for the GetTable part

sr=jst
Attachment #155960 - Flags: superreview?(jst) → superreview+
Checked in that part... Leaving open for further work.
Component: DOM: HTML → DOM: Core & HTML
QA Contact: ian → general
Testcase WFM.
Assignee: general → nobody
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: html5, testcase
Resolution: --- → WORKSFORME
The attached testcase only tests part of the bug.  Please read the bug before resolving...
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Worksforme now, I guess, modulo the bugginess or .rows.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 11 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: