[Webkit-unassigned] [Bug 14848] DOM table rules are not updated when changed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 13 11:51:36 PDT 2007


darin at apple.com changed:

           What    |Removed                     |Added
  Attachment #15878|review?                     |review+
               Flag|                            |

------- Comment #5 from darin at apple.com  2007-08-13 11:51 PDT -------
(From update of attachment 15878)
I think setTableCellsChanged could be written so it's not recursive, but I
guess it's OK as-is. I do worry about adding more and more recursive algorithms
that could be a problem if someone makes a deeply nested DOM tree, but Hyatt
tells me not to worry.

+    bool result = false;

I think "cellChanged" might be a better name for this local variable. Also, I'd
put a space after this to match the paragraphing of the rest of the function.

+    if (n->hasTagName(theadTag) || n->hasTagName(tbodyTag) ||
+        n->hasTagName(tfootTag) || n->hasTagName(trTag) ||
+         n->hasTagName(thTag)) {

I'd prefer a helper function for this (inline if necessary) -- it's hard to
read a long list like this.

+        for (Node *child = n->firstChild();child;child = child->nextSibling())
+            result |= setTableCellsChanged(child);
+        }

There should not be space between Node and the * after it. There should be
spaces after the semicolons. There should not be braces around the single line
block inside the loop.

+    } else if (n->hasTagName(tdTag)) {
+        result = true;
+    }

We normally don't put braces around single line statements like this one.
Putting the tdTag case first before the other tags might make the formatting
easier to read.

What about cases where we don't have actual <td> elements, but rather things
styled as table-cell by CSS. Does this bug happen in that case?

r=me anyway, even though I had a lot of questions.

Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

More information about the webkit-unassigned mailing list