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

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=14848


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.