[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 #156642|review?                     |
               Flag|                            |




--- Comment #15 from Julien Chaffraix <jchaffraix at webkit.org>  2012-08-06 10:38:59 PST ---
(From update of attachment 156642)
View in context: https://bugs.webkit.org/attachment.cgi?id=156642&action=review

hittest-tablecell-edge.html is a subset of hittest-tablecell-edge-empty-columns.html. Do we really need those 2 cases or would hittest-tablecell-edge-empty-columns.html be enough?

I would support landing a test with borders on the table-cells similar to hittest-tablecell-edge-empty-columns.html.

Also, it would be nice to do testing on several rows (it shouldn't be impacted by your change but that should be easy to add as part of your change).

Due to the tabs, this change will need another round but it looks good.

> Source/WebCore/rendering/RenderTableSection.cpp:1122
> +    unsigned nextColumn = std::upper_bound(columnPos.begin(), columnPos.end(), flippedRect.x()) - columnPos.begin();

Please amend the FIXME in spannedRows as it's not true anymore. Don't forget to mention that this matches other browsers.

> LayoutTests/fast/dom/hittest-tablecell-edge.html:21
> +	['td1', 'td2', 'td3'].forEach(function(a) {
> +		ele[a] = document.getElementById(a);
> +	});
> +	
> +	['td1', 'td2', 'td3'].forEach(function(a) {
> +		hittest(ele[a], a);
> +	});
> +	
> +	['td1', 'td2', 'td3'].forEach(function(a) {
> +		ele[a].innerHTML = '';
> +	});

We can maybe fold all those forEach into one?

> LayoutTests/fast/dom/hittest-tablecell-edge.html:32
> +	y = ele.getBoundingClientRect().bottom/2;
> +	debug('Executing for element: '+orgElement);
> +	debug('Node at edge-1: '+document.elementFromPoint(x-1,y).id);
> +	debug('Node at edge: '+document.elementFromPoint(x,y).id);
> +	debug('Node at edge+1: '+document.elementFromPoint(x+1,y).id);

It's prefered to use WebKit style for tests.

> LayoutTests/fast/dom/hittest-tablecell-edge.html:51
> +			<td id="td3">1</td>

Those are tabs, your patch will be rejected by our commit-hook.

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