Use display:block for dialog in UA stylesheet.

Use display:block for dialog in UA stylesheet.

This patch adds display:block for dialog to the UA stylesheet. This
modification ensures that all the tests in [3] pass.

More Information:
[1] - https://github.com/whatwg/html/pull/2458
[2] - https://github.com/w3c/web-platform-tests/pull/5211

[3] - http://w3c-test.org/html/rendering/non-replaced-elements/flow-content-0/dialog-display.html

BUG=704848
Review-Url: https://codereview.chromium.org/2782823005
Cr-Commit-Position: refs/heads/master@{#460665}
Committed: https://chromium.googlesource.com/chromium/src/+/0799d3367209d4185480b0408397eea79cd67fc8

[MartinRogalla, 2017-03-29 23:07:25]

martin@martinrogalla.com changed reviewers:
+ tkent@chromium.org

[tkent, 2017-03-29 23:16:05]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 23:17:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 00:36:39]

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

[commit-bot: I haz the power, 2017-03-30 00:36:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2017-03-30 00:42:51]

The test html/dialog/element-removed-from-top-layer-has-original-position.html
fails with Patch Set 1.  It's an expected behavior change.
You should add a following line to somewhere (not bottom) in
LayoutTests/TestExpectations

crbug.com/704848
html/dialog/element-removed-from-top-layer-has-original-position.html [
NeedsRebaseline ]

[MartinRogalla, 2017-03-30 00:57:17]

On 2017/03/30 00:42:51, tkent wrote:
> The test html/dialog/element-removed-from-top-layer-has-original-position.html
> fails with Patch Set 1.  It's an expected behavior change.
> You should add a following line to somewhere (not bottom) in
> LayoutTests/TestExpectations
> 
> crbug.com/704848
> html/dialog/element-removed-from-top-layer-has-original-position.html [
> NeedsRebaseline ]

When running git cl upload I get the following Presubmit Error:
LayoutTests/TestExpectations:803 A reftest without text expectation cannot be
marked as NeedsRebaseline/NeedsManualRebaseline
html/dialog/element-removed-from-top-layer-has-original-position.html

[tkent, 2017-03-30 00:59:39]

On 2017/03/30 at 00:57:17, martin wrote:
> When running git cl upload I get the following Presubmit Error:
> LayoutTests/TestExpectations:803 A reftest without text expectation cannot be
marked as NeedsRebaseline/NeedsManualRebaseline
html/dialog/element-removed-from-top-layer-has-original-position.html

Oh, it's a ref-test, not a pixel test.  Sorry for the wrong advice.

You should update
html/dialog/element-removed-from-top-layer-has-original-position-expected.html. 
Maybe replacing the second <span> with <div> would work.

[MartinRogalla, 2017-03-30 01:09:53]

On 2017/03/30 00:59:39, tkent wrote:
> On 2017/03/30 at 00:57:17, martin wrote:
> > When running git cl upload I get the following Presubmit Error:
> > LayoutTests/TestExpectations:803 A reftest without text expectation cannot
be
> marked as NeedsRebaseline/NeedsManualRebaseline
> html/dialog/element-removed-from-top-layer-has-original-position.html
> 
> Oh, it's a ref-test, not a pixel test.  Sorry for the wrong advice.
> 
> You should update
>
html/dialog/element-removed-from-top-layer-has-original-position-expected.html. 
> Maybe replacing the second <span> with <div> would work.

That doesn't seem to work for me. Using the (now) default 'display:block' would
always cause the dialog to fall to the next line. Would adding 'display:inline'
to #right-dialog work, since the test is purely trying to test the behavior of
the position property? I'm checking this with you, because I'm not sure if it
would still test for the same bug that was mentioned in the test.

[tkent, 2017-03-30 01:13:36]

> > You should update
> >
html/dialog/element-removed-from-top-layer-has-original-position-expected.html. 
> > Maybe replacing the second <span> with <div> would work.
> 
> That doesn't seem to work for me. Using the (now) default 'display:block'
would always cause the dialog to fall to the next line. Would adding
'display:inline' to #right-dialog work, since the test is purely trying to test
the behavior of the position property? I'm checking this with you, because I'm
not sure if it would still test for the same bug that was mentioned in the test.

You're right. It tests an inline element as a top layer.  So, we should update
html/dialog/element-removed-from-top-layer-has-original-position.html so that
the <dialog> has display:inline.

[MartinRogalla, 2017-03-30 01:17:51]

On 2017/03/30 01:13:36, tkent wrote:
> > > You should update
> > >
>
html/dialog/element-removed-from-top-layer-has-original-position-expected.html. 
> > > Maybe replacing the second <span> with <div> would work.
> > 
> > That doesn't seem to work for me. Using the (now) default 'display:block'
> would always cause the dialog to fall to the next line. Would adding
> 'display:inline' to #right-dialog work, since the test is purely trying to
test
> the behavior of the position property? I'm checking this with you, because I'm
> not sure if it would still test for the same bug that was mentioned in the
test.
> 
> You're right. It tests an inline element as a top layer.  So, we should update
> html/dialog/element-removed-from-top-layer-has-original-position.html so that
> the <dialog> has display:inline.

Okay, thanks. I modified the test.

[tkent, 2017-03-30 01:19:26]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 01:20:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 04:28:12]

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

[commit-bot: I haz the power, 2017-03-30 04:28:13]

Dry run: This issue passed the CQ dry run.

[tkent, 2017-03-30 04:37:43]

The CQ bit was checked by tkent@chromium.org

[tkent, 2017-03-30 04:37:43]

lgtm

[commit-bot: I haz the power, 2017-03-30 04:37:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 04:42:38]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490848663412180,
"parent_rev": "1582bcfa6979192f20e21d3ae4afb6951f0793c6", "commit_rev":
"0799d3367209d4185480b0408397eea79cd67fc8"}

[commit-bot: I haz the power, 2017-03-30 04:43:24]

Description was changed from

==========
Use display:block for dialog in UA stylesheet.

This patch adds display:block for dialog to the UA stylesheet. This
modification ensures that all the tests in [3] pass.

More Information:
[1] - https://github.com/whatwg/html/pull/2458
[2] - https://github.com/w3c/web-platform-tests/pull/5211

[3] -
http://w3c-test.org/html/rendering/non-replaced-elements/flow-content-0/dialo...

BUG=704848
==========

to

==========
Use display:block for dialog in UA stylesheet.

This patch adds display:block for dialog to the UA stylesheet. This
modification ensures that all the tests in [3] pass.

More Information:
[1] - https://github.com/whatwg/html/pull/2458
[2] - https://github.com/w3c/web-platform-tests/pull/5211

[3] -
http://w3c-test.org/html/rendering/non-replaced-elements/flow-content-0/dialo...

BUG=704848

Review-Url: https://codereview.chromium.org/2782823005
Cr-Commit-Position: refs/heads/master@{#460665}
Committed:
https://chromium.googlesource.com/chromium/src/+/0799d3367209d4185480b0408397...
==========

[commit-bot: I haz the power, 2017-03-30 04:43:25]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0799d3367209d4185480b0408397...