[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





--- Comment #17 from Arpita Bahuguna <arpitabahuguna at gmail.com>  2012-08-07 07:27:40 PST ---
(In reply to comment #15)
> (From update of attachment 156642 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156642&action=review

Thank-you for the review Julien. Have uploaded another patch as per your comments.
> 
> 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).

Have now added 4 testcases, all containing empty table cells and multiple rows, that carry out the hittest on the edge of adjoining table cells, both on the right edge as well as on the bottom edge (this has been added just to verify the rows behavior).

Also, have added separate testcases that have borders specified for the table-cells to highlight the difference in the hittesting results on the edge, with and without borders.

Please note that for all these cases, our behavior is now exactly similar to that of Opera and Firefox.


> Please amend the FIXME in spannedRows as it's not true anymore. Don't forget to mention that this matches other browsers.
> 
Have removed the FIXME in spannedRows and also added a comment in spannedColumns.

> > 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?
> 
Done.

> > 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.
> 
Changed these to use shouldBe() instead.

> > 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.
I apologize for this oversight. Shall take care of this in the future.

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