Should not render the table cell border if the <td> is empty.

Should not render the table cell border if the <td> is empty.

When table cell has no children and the doctype is not present or not
correct the border for that cell should not be drawn. Firefox also
follows the same behaviour hence making the changes to make it similiar
to Firefox behaviour.

Added condition for not drawing the border when table cell has no child
and doctype.

I already landed this patch on webkit. http://trac.webkit.org/changeset/162334.

R=jchaffraix@chromium.org
BUG=353483

[Gurpreet, 2014-03-18 14:36:58]

Hi Julien. Can you please review this? Thanks.

[Julien - ping for review, 2014-03-19 00:41:58]

On 2014/03/18 14:36:58, Gurpreet wrote:
> Hi Julien. Can you please review this? Thanks.

Thanks for the patch, however I don't think we should be fixing any quirks mode
bugs. Quirks mode is a legacy code path that we would get rid of if not for its
use.

The only pages that I would expect to be in quirks mode nowadays are very old
content or pages from deeply confused people. Neither of which justifies making
our code base more complex and opening us to the potential "regressions" as we
change behaviors.

[Gurpreet, 2014-03-19 07:36:34]

On 2014/03/19 00:41:58, Julien Chaffraix - PST wrote:
> On 2014/03/18 14:36:58, Gurpreet wrote:
> > Hi Julien. Can you please review this? Thanks.
> 
> Thanks for the patch, however I don't think we should be fixing any quirks
mode
> bugs. Quirks mode is a legacy code path that we would get rid of if not for
its
> use.
> 
> The only pages that I would expect to be in quirks mode nowadays are very old
> content or pages from deeply confused people. Neither of which justifies
making
> our code base more complex and opening us to the potential "regressions" as we
> change behaviors.

Hi Julien. Thanks for reviewing. But still so many places we have code checking
for quirks mode :(

[esprehn, 2014-03-19 22:22:33]

This patch lgtm to me. It's really simple and consistency between browsers is
important. Given that Firefox and WebKit behave this way now, I think we should
too. I also appreciate the tests.

Are you okay if we land this Julien?

[Julien - ping for review, 2014-03-20 00:44:25]

On 2014/03/19 22:22:33, esprehn wrote:
> This patch lgtm to me. It's really simple and consistency between browsers is
> important. Given that Firefox and WebKit behave this way now, I think we
should
> too. I also appreciate the tests.
> 
> Are you okay if we land this Julien?

I still think it's a waste of everyone's time to fix these bugs. It also adds
more unwarranted complexity to our code. Here it's a small change that aligns us
with at least 3 engines so I won't block it.

[esprehn, 2014-03-20 03:12:54]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2014-03-20 03:13:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/k.gurpreet@samsung.com/203473002/1

[commit-bot: I haz the power, 2014-03-20 03:45:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 03:45:07]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_rel

[esprehn, 2014-03-20 03:58:59]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2014-03-20 03:59:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/k.gurpreet@samsung.com/203473002/1

[esprehn, 2014-03-20 03:59:19]

The CQ bit was unchecked by esprehn@chromium.org

[esprehn, 2014-03-20 03:59:25]

This seems to make tons of tests fail.

[Gurpreet, 2014-03-20 04:01:17]

On 2014/03/20 03:59:25, esprehn wrote:
> This seems to make tons of tests fail.

Hi will check.

[Gurpreet, 2014-03-20 04:35:22]

On 2014/03/20 04:01:17, Gurpreet wrote:
> On 2014/03/20 03:59:25, esprehn wrote:
> > This seems to make tons of tests fail.
> 
> Hi will check.

Its surprising though because when I checked-in the patch in Webkit none of the
test case failed :(

[Gurpreet, 2014-03-20 05:21:34]

On 2014/03/20 04:35:22, Gurpreet wrote:
> On 2014/03/20 04:01:17, Gurpreet wrote:
> > On 2014/03/20 03:59:25, esprehn wrote:
> > > This seems to make tons of tests fail.
> > 
> > Hi will check.
> 
> Its surprising though because when I checked-in the patch in Webkit none of
the
> test case failed :(

I checked few failed test cases and the expected result is now different after
the change and the actual behavior is matching with Firefox.    The failed test
cases do not have the <!DOCTYPE html> mentioned so its failing.

[Gurpreet, 2014-03-24 12:11:19]

Please suggest. Shall I fix all the failed test cases?

[Gurpreet, 2014-04-01 14:41:39]

Hello. Whats your thought on this?

[Gurpreet, 2014-04-30 07:57:41]

Hello. Please suggest else shall I drop this ?

[Julien - ping for review, 2014-05-01 19:22:16]

On 2014/04/30 07:57:41, Gurpreet wrote:
> Hello. Please suggest else shall I drop this ?

I made my viewpoint about the change clear so I won't add more to that.

You have to look at the failures and determine if those are regressions or
progressions. If they are regressions, you will need to rethink your change. If
not, it's just a matter of adding new baselines (and/or NeedsRebaseline entries
in LayoutTests/TestExpectations).

[Gurpreet, 2014-05-05 09:38:49]

Hi Julien. I understand your viewpoint. The test cases are failing since it
doesnot have <!DOCTYPE html> and we have made changes based on that.

[Julien - ping for review, 2014-05-06 01:02:50]

On 2014/05/05 09:38:49, Gurpreet wrote:
> Hi Julien. I understand your viewpoint. The test cases are failing since it
> doesnot have <!DOCTYPE html> and we have made changes based on that.

You have 2 choices:
- add a DOCTYPE, which means changing the quirks mode coverage we have but
getting more non-quirks mode coverage.
- update the baselines after checking that the changes are neutral /
progressions (you have to compare to what other browsers are doing).

I would be wary about changing some test coverage without thinking about it
really hard.

[Gurpreet, 2014-05-16 11:33:34]

The CQ bit was checked by k.gurpreet@samsung.com

[commit-bot: I haz the power, 2014-05-16 11:33:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/k.gurpreet@samsung.com/203473002/1

[commit-bot: I haz the power, 2014-05-16 12:52:52]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/7578)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/7352)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/6868)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/7348)

[commit-bot: I haz the power, 2014-05-16 13:40:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-16 13:40:04]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/6885)

[Gurpreet, 2015-06-22 08:44:40]

Currently not working on this issue. If still reproducible on latest and someone
interested to work can take it up.
Thanks.