Bug 3295 - cellIndex is always zero
Summary: cellIndex is always zero
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P3 Normal
Assignee: Justin Garcia
URL:
Keywords:
: 3623 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-06-07 05:54 PDT by Jonathan Hurshman
Modified: 2005-07-13 23:13 PDT (History)
2 users (show)

See Also:


Attachments
A testcase for this bug (675 bytes, text/html)
2005-06-07 05:57 PDT, Jonathan Hurshman
no flags Details
Proposed patch (2.52 KB, patch)
2005-06-17 13:42 PDT, Justin Garcia
hyatt: review+
Details | Formatted Diff | Diff
Proposed patch with better coding style :-) (2.52 KB, patch)
2005-06-17 14:30 PDT, Justin Garcia
hyatt: review+
Details | Formatted Diff | Diff
Remove outdated commnet (522 bytes, patch)
2005-06-18 03:02 PDT, Harri Porten
no flags Details | Formatted Diff | Diff
Remove outdated comment (522 bytes, patch)
2005-06-18 03:02 PDT, Harri Porten
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Hurshman 2005-06-07 05:54:57 PDT
The cellIndex property of TD nodes is always 0, regardless of the actual position of the cell in the row.
Comment 1 Jonathan Hurshman 2005-06-07 05:57:27 PDT
Created attachment 2128 [details]
A testcase for this bug
Comment 2 Jonathan Hurshman 2005-06-07 06:14:28 PDT
(This bug was copied from Apple's Bug Reporter #3756860)
Comment 3 Allan Sandfeld Jensen 2005-06-13 03:47:53 PDT
Fixed in Konqueror 
Comment 5 Justin Garcia 2005-06-17 13:42:32 PDT
Created attachment 2448 [details]
Proposed patch
Comment 6 Dave Hyatt 2005-06-17 14:09:36 PDT
+    const NodeImpl * node = this;
+    int index = 0;
+    do {
+        node = node->previousSibling();
+        if(node && (node->id() == ID_TD || node->id() == ID_TH))
+            index++;
+    } while(node);

for (const NodeImpl* node = node->previousSibling(); node; node = node->previousSibling())
    if (node->id() == ID_TH || node->id() == ID_TH)
        index++;

... reads better to me then a do construct that still ends up null-checking.
Comment 7 Dave Hyatt 2005-06-17 14:10:09 PDT
+    const NodeImpl * node = this;
+    int index = 0;
+    do {
+        node = node->previousSibling();
+        if(node && (node->id() == ID_TD || node->id() == ID_TH))
+            index++;
+    } while(node);

for (const NodeImpl* node = node->previousSibling(); node; node = node->previousSibling()) {
    if (node->id() == ID_TD || node->id() == ID_TH)
        index++;
}

... reads better to me then a do construct that still ends up null-checking.
Comment 8 Dave Hyatt 2005-06-17 14:10:59 PDT
node = previousSibling().

Yes, I can write code. Really. ;)
Comment 9 Dave Hyatt 2005-06-17 14:14:40 PDT
Comment on attachment 2448 [details]
Proposed patch

r=me, would prefer for loop, but don't really feel strongly. :)
Comment 10 Justin Garcia 2005-06-17 14:30:42 PDT
Created attachment 2449 [details]
Proposed patch with better coding style :-)
Comment 11 Dave Hyatt 2005-06-17 14:34:53 PDT
Comment on attachment 2449 [details]
Proposed patch with better coding style :-)

r=me, fix the 8-space indentation to be 4-space though.
Comment 12 Harri Porten 2005-06-18 03:02:43 PDT
Created attachment 2456 [details]
Remove outdated commnet

I don't think the comment is valid anymore.
Comment 13 Harri Porten 2005-06-18 03:02:47 PDT
Created attachment 2457 [details]
Remove outdated comment

I don't think the comment is valid anymore.
Comment 14 Darin Adler 2005-06-18 21:02:17 PDT
Patch looks good, and since Justin has commit privileges I'm not going to land it for him. Justin, you 
should commit this!
Comment 15 Darin Adler 2005-06-19 11:57:24 PDT
The for loop idiom is pretty nice, nicer than the original do/while. There's also a good while idiom for this 
sort of thing that you can see in functions like HTMLOptionElementImpl::text.
Comment 16 Anders Carlsson 2005-06-24 08:16:13 PDT
*** Bug 3623 has been marked as a duplicate of this bug. ***
Comment 17 Joost de Valk (AlthA) 2005-07-13 23:05:31 PDT
Fixed in 10.4.2