[Webkit-unassigned] [Bug 74864] There is additional space not belonged to a table between the table cells

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=74864


Julien Chaffraix <jchaffraix at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #157438|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #23 from Julien Chaffraix <jchaffraix at webkit.org>  2012-08-09 18:12:07 PST ---
(From update of attachment 157438)
View in context: https://bugs.webkit.org/attachment.cgi?id=157438&action=review

> Source/WebCore/ChangeLog:17
> +               fast/dom/hittest-tablecell-with-borders-right-edge.html

I didn't notice that those tests were in fast/dom. It doesn't make sense to put them there - you are not testing a dom API - and they would be better with the other table tests in fast/tables. If you prefer you could even put them in a sub-directory (fast/table/testing).

> Source/WebCore/rendering/RenderTableSection.cpp:1129
> +    // of other browsers.

Woua, that's a long explanation! It's also half true as spanningRows / spanningColumns are used for hit testing but also painting.

Let's try to shorten the comment (feel free to adapt):
// lower_bound doesn't handle the edge between 2 cells properly as it would wrongly return the cell on the logical top / left.
// upper_bound properly return the cell on the logical bottom / right, which matches other browsers.

> LayoutTests/fast/dom/hittest-tablecell-bottom-edge-expected.txt:41
> +PASS document.elementFromPoint(middleX, edge - 1).id is ''

This result is interesting. Note that this means that there is a 1px gap (or more) around an empty rows. This matches the empty column case too. As this patch makes us only progress, I am in favor of filing a follow-up bug about that.

> LayoutTests/fast/dom/hittest-tablecell-with-borders-right-edge-expected.txt:47
> +PASS document.elementFromPoint(edge - 1, middleY).id is ''
> +PASS document.elementFromPoint(edge, middleY).id is ''
> +PASS document.elementFromPoint(edge + 1, middleY).id is ''

I don't think this is fine to return nothing in all those cases but you are just validating our existing behavior here.

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