Don't force display:inline-table/table on table elements when in quirks mode

Don't force display:inline-table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline-table
or table. This current action is not supported in quirks spec and is
inconsistent with Gecko. This leads to elements with display set to
lose that information. This patch fixes that and allows those elements
with display to retain this state even in quirks mode.

BUG=369979
Committed: https://crrev.com/0aea8a9a12d5a3b60bb022953825f900f90e7b4b
Cr-Commit-Position: refs/heads/master@{#370382}

[nainar, 2016-01-19 05:40:26]

nainar@chromium.org changed reviewers:
+ rune@opera.com

[nainar, 2016-01-19 05:40:27]

@rune, 

PTAL? 

Thanks!

[rune, 2016-01-19 09:54:51]

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/TestExpectations:961: crbug.com/369979
fast/encoding/utf-16-little-endian.html [ NeedsRebaseline ]
I suggest you change display:inline to display:inline-table in those test-cases
to avoid the rebaseline.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/dom/inner-text-001-expected.txt
(right):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/dom/inner-text-001-expected.txt:38: And an
inline	table	with	rows not far behind.
Change display:inline -> display:inline-table in test.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/table/quirks-mode-ignore-display-inline-table.html
(right):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/table/quirks-mode-ignore-display-inline-table.html:8:
assert_equals(getComputedStyle(table).display, "inline");
You should test tbody, tr, td, ... as well. And also on other display type.
Block, for instance.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/table/quirks-mode-ignore-display-inline-table.html:9:
}, "Test to make sure that in quirks mode table elements with display:inline act
as they do in standard mode and retain display:inline");
Not only display:inline, but display in general?

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp (left):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp:369:
style.setDisplay(style.isDisplayInlineType() ? INLINE_TABLE : TABLE);
We should just drop this whole quirk altogether, I think.

[nainar, 2016-01-19 10:44:11]

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp (left):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp:369:
style.setDisplay(style.isDisplayInlineType() ? INLINE_TABLE : TABLE);
I looked up the quirks spec - this isn't mentioned there. Firefox doesn't have
it either. Will change patch to remove this quirk altogether.

[nainar, 2016-01-20 00:41:06]

Description was changed from

==========
Don't ignore display:inline on table elements when in quirks mode

Currently in quirks mode we force <td>s and <table>s to retain their
display types. This leads to elements with "display: inline" being
forced to "display: inline-table." This patch fixes that and allows
those elements with "display:inline" to retain this state even in
quirks mode.

BUG=369979
==========

to

==========
Don't force display:inline_table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline_table
or table. This current action is not supported in quirks spec and is 
inconsistent with Gecko. This leads to elements with display set to 
lose that information. This patch fixes that and allows those elements 
with display to retain this state even in quirks mode.

BUG=369979
==========

[nainar, 2016-01-20 00:44:23]

@rune,

PTAL? Thanks!

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/TestExpectations:961: crbug.com/369979
fast/encoding/utf-16-little-endian.html [ NeedsRebaseline ]
Done for all possible tests. Some still need to be rebaselined.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/dom/inner-text-001-expected.txt
(right):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/dom/inner-text-001-expected.txt:38: And an
inline	table	with	rows not far behind.
Done.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/table/quirks-mode-ignore-display-inline-table.html
(right):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/table/quirks-mode-ignore-display-inline-table.html:8:
assert_equals(getComputedStyle(table).display, "inline");
Done.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/table/quirks-mode-ignore-display-inline-table.html:9:
}, "Test to make sure that in quirks mode table elements with display:inline act
as they do in standard mode and retain display:inline");
Done.

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp (left):

https://codereview.chromium.org/1607703003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp:369:
style.setDisplay(style.isDisplayInlineType() ? INLINE_TABLE : TABLE);
Done.

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:958: crbug.com/369979
fast/table/table-display-types-vertical.html [ NeedsRebaseline ]
Not editing the two fast/table tests here as I think it would lose the purpose
of the test then.

[rune, 2016-01-20 10:47:05]

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:957: crbug.com/369979
fast/encoding/utf-16-little-endian.html [ NeedsRebaseline ]
Rietveld says the changes to these tests are binary, so I don't see what you
changed there, but shouldn't those changes remove the need for the rebaselines?

Changing the table display from inline to inline-table in utf-16-big-endian.html
for me locally didn't require a different reference.

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:958: crbug.com/369979
fast/table/table-display-types-vertical.html [ NeedsRebaseline ]
On 2016/01/20 00:44:23, nainar wrote:
> Not editing the two fast/table tests here as I think it would lose the purpose
> of the test then. 

Acknowledged.

[nainar, 2016-01-20 11:15:36]

@rune, 

The fast/encoding files should not have shown in the diff, I didn't change
either of them. Fixed now. PTAL? 

Thanks!

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:957: crbug.com/369979
fast/encoding/utf-16-little-endian.html [ NeedsRebaseline ]
This is weird. I made no changes to this file. Setting the display to
inline-table still needed a rebaseline. So I undid that change and am retaining
the tests as are.

[rune, 2016-01-20 11:56:52]

On 2016/01/20 11:15:36, nainar wrote:
> @rune, 
> 
> The fast/encoding files should not have shown in the diff, I didn't change
> either of them. Fixed now. PTAL? 

I think you should change those tests. If that requires a rebaseline for those
tests, I think there's something funny is going on.

[rune, 2016-01-20 11:57:01]

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:957: crbug.com/369979
fast/encoding/utf-16-little-endian.html [ NeedsRebaseline ]
On 2016/01/20 11:15:36, nainar wrote:
> This is weird. I made no changes to this file. Setting the display to
> inline-table still needed a rebaseline. So I undid that change and am
retaining
> the tests as are. 

That doesn't make sense to me. Why did the test-output change for these tests
when changing display from inline to inline-table when the previous code would
do exactly that adjustment during style resolving?

[nainar, 2016-01-20 12:35:45]

On 2016/01/20 at 11:57:01, rune wrote:
>
https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/TestExpectations (right):
> 
>
https://codereview.chromium.org/1607703003/diff/20001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/TestExpectations:957: crbug.com/369979
fast/encoding/utf-16-little-endian.html [ NeedsRebaseline ]
> On 2016/01/20 11:15:36, nainar wrote:
> > This is weird. I made no changes to this file. Setting the display to
> > inline-table still needed a rebaseline. So I undid that change and am
retaining
> > the tests as are. 
> 
> That doesn't make sense to me. Why did the test-output change for these tests
when changing display from inline to inline-table when the previous code would
do exactly that adjustment during style resolving?

1. That was my silly mistake I was forcing a span element to
display:inline-table as well. Which was causing the difference. 

2. I can't seem to get the diff for these two files though
(utf-16-[big/little]-endian.html), Code Search also shows them as binaries. 

The diff should read:
Change to line 9 of both files:
- <TABLE style="display:inline;" CELLSPACING="0">
+ <TABLE style="display:inline-table;" CELLSPACING="0">

Also can't place a comment in the binary file itself.

[rune, 2016-01-20 12:40:16]

lgtm

[rune, 2016-01-20 12:41:57]

On 2016/01/20 12:40:16, rune wrote:
> lgtm

Could you change "display:inline_table" to "display:inline-table" in the
description before landing?

[nainar, 2016-01-20 12:43:41]

Description was changed from

==========
Don't force display:inline_table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline_table
or table. This current action is not supported in quirks spec and is 
inconsistent with Gecko. This leads to elements with display set to 
lose that information. This patch fixes that and allows those elements 
with display to retain this state even in quirks mode.

BUG=369979
==========

to

==========
Don't force display:inline-table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline_table
or table. This current action is not supported in quirks spec and is 
inconsistent with Gecko. This leads to elements with display set to 
lose that information. This patch fixes that and allows those elements 
with display to retain this state even in quirks mode.

BUG=369979
==========

[nainar, 2016-01-20 12:43:49]

On 2016/01/20 at 12:41:57, rune wrote:
> On 2016/01/20 12:40:16, rune wrote:
> > lgtm
> 
> Could you change "display:inline_table" to "display:inline-table" in the
description before landing?

Done.

[nainar, 2016-01-20 12:44:11]

Description was changed from

==========
Don't force display:inline-table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline_table
or table. This current action is not supported in quirks spec and is 
inconsistent with Gecko. This leads to elements with display set to 
lose that information. This patch fixes that and allows those elements 
with display to retain this state even in quirks mode.

BUG=369979
==========

to

==========
Don't force display:inline-table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline-table
or table. This current action is not supported in quirks spec and is 
inconsistent with Gecko. This leads to elements with display set to 
lose that information. This patch fixes that and allows those elements 
with display to retain this state even in quirks mode.

BUG=369979
==========

[nainar, 2016-01-20 12:44:28]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2016-01-20 12:45:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1607703003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1607703003/60001

[commit-bot: I haz the power, 2016-01-20 14:16:40]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-20 14:18:04]

Description was changed from

==========
Don't force display:inline-table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline-table
or table. This current action is not supported in quirks spec and is 
inconsistent with Gecko. This leads to elements with display set to 
lose that information. This patch fixes that and allows those elements 
with display to retain this state even in quirks mode.

BUG=369979
==========

to

==========
Don't force display:inline-table/table on table elements when in quirks mode

Currently in quirks mode we force table elements to display:inline-table
or table. This current action is not supported in quirks spec and is
inconsistent with Gecko. This leads to elements with display set to
lose that information. This patch fixes that and allows those elements
with display to retain this state even in quirks mode.

BUG=369979

Committed: https://crrev.com/0aea8a9a12d5a3b60bb022953825f900f90e7b4b
Cr-Commit-Position: refs/heads/master@{#370382}
==========

[commit-bot: I haz the power, 2016-01-20 14:18:04]

Patchset 4 (id:??) landed as
https://crrev.com/0aea8a9a12d5a3b60bb022953825f900f90e7b4b
Cr-Commit-Position: refs/heads/master@{#370382}