[webkit-reviews] review requested: [Bug 35422] [Gtk] Layout tables should indicate that they are not data tables via an object attribute : [Attachment 71628] Patch proposal + Unit test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Oct 23 01:02:13 PDT 2010


Mario Sanchez Prada <msanchez at igalia.com> has asked  for review:
Bug 35422: [Gtk] Layout tables should indicate that they are not data tables
via an object attribute
https://bugs.webkit.org/show_bug.cgi?id=35422

Attachment 71628: Patch proposal + Unit test
https://bugs.webkit.org/attachment.cgi?id=71628&action=review

------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
I'm attaching a patch that would fix this issue, along with an unit test to
check it's behaviour.

The ChangeLog entry included is pretty descriptive and extense so I'd just like
to explain here why I didn't include a Layout test:

  1. The code I'm changing doesn't really add new functionality other than
splitting the behavior of the former isDataTable() function in two:
isAccessibilityTable() and a more coherent version of the former isDataTable()
function. It's more like a reorganization of the code to allow asking those two
questions separately, where the new isAccessibilityTable() function is
basically the same than the old isDataTable() in all the ports but the GTK one.


  2. Such differentiation is only used now in the GTK port (the patch makes
sure the rest of the ports keep working exactly as usual), and that change is
tested by the unit test provided along with this patch.

  3. I ran all the Layout tests with this patch and they keep passing,
confirming the former behaviour keeps working (after replacing isDataTable()
with isAccessibilityTable() where needed).

Because of these reasons, I thought a layout test was not actually needed. Now
head to the ChangeLog entry if you want more, deeper, details on this patch.

Have a nice weekend!


More information about the webkit-reviews mailing list