Add tests for dialog centering #4605

domenic · 2017-01-23T21:55:13Z

Several of these are probably wrong but I feel I've gone as far as I can with my knowledge of writing modes. @zcorpan can you take this over from here?

This change is

wpt-pr-bot · 2017-01-23T21:55:24Z

Notifying @Ms2ger, @ayg, @gsnedders, @jdm, @jgraham, @sideshowbarker, @zcorpan, and @zqzhang. (Learn how reviewing works.)

wpt-stability-bot · 2017-01-23T22:00:35Z

Firefox (nightly channel)

Testing web-platform-tests at revision c38f204
Using browser at version BuildID 20170123125947; SourceStamp 36486fdc3813ef7943ae5b07b4128866d1938a6c
Starting 10 test iterations
All results were stable

All results

/html/semantics/interactive-elements/the-dialog-element/centering.html

Subtest	Results
	OK
`vertical-rl (dialog/dialog container horizontal-tb): tall viewport`	PASS
`horizontal-tb: tall viewport`	FAIL
`horizontal-tb: square viewport`	FAIL
`horizontal-tb: wide viewport`	FAIL
`horizontal-tb: dialog and viewport match`	FAIL
`vertical-lr: dialog bigger than viewport`	FAIL
`horizontal-tb (dialog/dialog container vertical-rl): tall viewport`	PASS
`vertical-rl: tall viewport`	FAIL
`horizontal-tb: dialog bigger than viewport`	FAIL
`vertical-lr: tall viewport`	FAIL

wpt-stability-bot · 2017-01-23T22:00:53Z

Chrome (unstable channel)

Testing web-platform-tests at revision c38f204
Using browser at version 55.0.2883.75
Starting 10 test iterations
All results were stable

All results

/html/semantics/interactive-elements/the-dialog-element/centering.html

Subtest	Results
	OK
`vertical-rl (dialog/dialog container horizontal-tb): tall viewport`	PASS
`horizontal-tb: tall viewport`	PASS
`horizontal-tb: square viewport`	PASS
`horizontal-tb: wide viewport`	PASS
`horizontal-tb: dialog and viewport match`	PASS
`vertical-lr: dialog bigger than viewport`	PASS
`horizontal-tb (dialog/dialog container vertical-rl): tall viewport`	FAIL
`vertical-rl: tall viewport`	FAIL
`horizontal-tb: dialog bigger than viewport`	PASS
`vertical-lr: tall viewport`	FAIL

zcorpan · 2017-01-24T08:52:11Z

html/semantics/interactive-elements/the-dialog-element/centering-iframe.sub.html

+    writing-mode: {{GET[html-writing-mode]}}
+  }
+
+  dialog, body {


We can't use body here because 'writing-mode' propagates from body to the viewport. Should use a div. Also don't need to put dialog in the selector since it's inherited.

zcorpan · 2017-01-24T08:57:25Z

html/semantics/interactive-elements/the-dialog-element/centering.html

+    iframe.height = iframeHeight;
+    iframe.onload = t.step_func_done(() => {
+      const dialog = iframe.contentDocument.querySelector("dialog");
+      assert_equals(window.getComputedStyle(dialog)[property], numericValue + "px");


I think this should probably invoke the iframe's getComputedStyle to avoid weirdness with crossing globals.

domenic · 2017-01-24T18:56:07Z

@zcorpan review comments addressed. Still hoping for your help on the TODOs :)

domenic · 2017-02-08T21:05:12Z

Ping @zcorpan

ghost · 2017-03-23T09:14:33Z

These tests are now available on w3c-test.org

zcorpan · 2017-03-23T09:20:16Z

Finally got around to this, sorry for the delay.

domenic · 2017-03-23T09:23:00Z

Great!! Your changes LGTM, hopefully my changes LGTY, so we can land this?

Travis is having trouble finding Firefox nightly... I restarted it to see what happens. Maybe rebasing would help too if the test infra has changed since the original PR.

zcorpan · 2017-03-23T09:24:18Z

Restarting won't help, need to rebase. (Edit: now done)

Follows whatwg/html#2208.

This seems like it might help avoid some false positives

ghost · 2017-03-23T09:36:07Z

View the complete job log.

Firefox (nightly channel)

Testing web-platform-tests at revision 190a5d5
Using browser at version BuildID 20170322110306; SourceStamp 201231223cd4354a450c3e5d80959f35b8e4cf0c
Starting 10 test iterations
All results were stable

All results

1 test ran

/html/semantics/interactive-elements/the-dialog-element/centering.html

Subtest	Results	Messages
	OK
`horizontal-tb: tall viewport`	FAIL	`assert_equals: expected "45px" but got "auto"`
`horizontal-tb: wide viewport`	FAIL	`assert_equals: expected "15px" but got "auto"`
`horizontal-tb: square viewport`	FAIL	`assert_equals: expected "45px" but got "auto"`
`horizontal-tb: dialog and viewport match`	FAIL	`assert_equals: expected "0px" but got "auto"`
`horizontal-tb: dialog bigger than viewport`	FAIL	`assert_equals: expected "0px" but got "auto"`
`vertical-rl: tall viewport`	FAIL	`assert_equals: expected "10px" but got "auto"`
`vertical-lr: tall viewport`	FAIL	`assert_equals: expected "10px" but got "auto"`
`vertical-lr: dialog bigger than viewport`	FAIL	`assert_equals: expected "0px" but got "auto"`
`vertical-rl (dialog horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-lr (dialog horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-lr (dialog horizontal-tb): dialog bigger than viewport`	PASS
`horizontal-tb (container vertical-rl): tall viewport`	FAIL	`assert_equals: expected "10px" but got "auto"`
`vertical-rl (container horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "45px" but got "auto"`
`horizontal-tb (container vertical-rl) (dialog horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-rl (container horizontal-tb) (dialog vertical-rl): tall viewport`	FAIL	`assert_equals: expected "45px" but got "0px"`

ghost · 2017-03-23T09:36:08Z

View the complete job log.

Chrome (unstable channel)

Testing web-platform-tests at revision 190a5d5
Using browser at version 59.0.3047.0 dev
Starting 10 test iterations
All results were stable

All results

1 test ran

/html/semantics/interactive-elements/the-dialog-element/centering.html

Subtest	Results	Messages
	OK
`horizontal-tb: tall viewport`	PASS
`horizontal-tb: wide viewport`	PASS
`horizontal-tb: square viewport`	PASS
`horizontal-tb: dialog and viewport match`	PASS
`horizontal-tb: dialog bigger than viewport`	PASS
`vertical-rl: tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-lr: tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-lr: dialog bigger than viewport`	PASS
`vertical-rl (dialog horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-lr (dialog horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-lr (dialog horizontal-tb): dialog bigger than viewport`	PASS
`horizontal-tb (container vertical-rl): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-rl (container horizontal-tb): tall viewport`	PASS
`horizontal-tb (container vertical-rl) (dialog horizontal-tb): tall viewport`	FAIL	`assert_equals: expected "10px" but got "0px"`
`vertical-rl (container horizontal-tb) (dialog vertical-rl): tall viewport`	PASS

zcorpan · 2017-03-23T09:53:54Z

Sigh, the spec's small-screen style is messing with this, since the iframe is small...

ghost · 2017-03-23T10:07:13Z

View the complete job log.

Firefox (nightly channel)

Testing web-platform-tests at revision 3b248100668aebdc5f58cca3135c0aecf31728d3
Using browser at version BuildID 20170322110306; SourceStamp 201231223cd4354a450c3e5d80959f35b8e4cf0c
Starting 10 test iterations

ghost · 2017-03-23T10:07:13Z

View the complete job log.

Chrome (unstable channel)

Testing web-platform-tests at revision 3b248100668aebdc5f58cca3135c0aecf31728d3
Using browser at version 59.0.3047.0 dev
Starting 10 test iterations

Fixes #2185. Tests: web-platform-tests/wpt#4605

Fixes whatwg#2185. Tests: web-platform-tests/wpt#4605

domenic added the do not merge yet label Jan 23, 2017

wpt-pr-bot added the html label Jan 23, 2017

zcorpan reviewed Jan 24, 2017

View reviewed changes

Ms2ger added the status:needs-spec-decision label Jan 31, 2017

zcorpan mentioned this pull request Mar 23, 2017

Use logical positioning for centering a dialog whatwg/html#2208

Merged

domenic and others added 4 commits March 23, 2017 10:25

Add tests for dialog centering

4a847b8

Follows whatwg/html#2208.

Address review feedback

086939b

Make dialog width/height different

71b0a16

This seems like it might help avoid some false positives

Use the container's writing mode to decide which dimension to use

21f3078

zcorpan force-pushed the dialog-centering branch from c363a98 to 21f3078 Compare March 23, 2017 09:25

zcorpan mentioned this pull request Mar 23, 2017

The UA stylesheet has a 540px "small devices" media query breakpoint (remove?) whatwg/html#624

Closed

zcorpan removed do not merge yet status:needs-spec-decision labels Apr 7, 2017

zcorpan merged commit 03b2ae7 into master Apr 7, 2017

zcorpan deleted the dialog-centering branch April 7, 2017 15:44

zcorpan pushed a commit to whatwg/html that referenced this pull request Apr 7, 2017

Use logical positioning for centering a dialog

a03a914

Fixes #2185. Tests: web-platform-tests/wpt#4605

inikulin pushed a commit to HTMLParseErrorWG/html that referenced this pull request May 9, 2017

Use logical positioning for centering a dialog

29cbd50

Fixes whatwg#2185. Tests: web-platform-tests/wpt#4605

inikulin pushed a commit to HTMLParseErrorWG/html that referenced this pull request May 9, 2017

Use logical positioning for centering a dialog

fffa206

Fixes whatwg#2185. Tests: web-platform-tests/wpt#4605

alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019

Use logical positioning for centering a dialog

2c82d3a

Fixes whatwg#2185. Tests: web-platform-tests/wpt#4605

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for dialog centering #4605

Add tests for dialog centering #4605

domenic commented Jan 23, 2017 •

edited by gsnedders

Loading

wpt-pr-bot commented Jan 23, 2017

wpt-stability-bot commented Jan 23, 2017 •

edited

Loading

wpt-stability-bot commented Jan 23, 2017 •

edited

Loading

zcorpan Jan 24, 2017

zcorpan Jan 24, 2017

domenic commented Jan 24, 2017

domenic commented Feb 8, 2017

ghost commented Mar 23, 2017 •

edited by ghost

Loading

zcorpan commented Mar 23, 2017

domenic commented Mar 23, 2017 •

edited

Loading

zcorpan commented Mar 23, 2017 •

edited

Loading

ghost commented Mar 23, 2017

ghost commented Mar 23, 2017

zcorpan commented Mar 23, 2017

ghost commented Mar 23, 2017

ghost commented Mar 23, 2017

Add tests for dialog centering #4605

Add tests for dialog centering #4605

Conversation

domenic commented Jan 23, 2017 • edited by gsnedders Loading

wpt-pr-bot commented Jan 23, 2017

wpt-stability-bot commented Jan 23, 2017 • edited Loading

Firefox (nightly channel)

All results

wpt-stability-bot commented Jan 23, 2017 • edited Loading

Chrome (unstable channel)

All results

zcorpan Jan 24, 2017

Choose a reason for hiding this comment

zcorpan Jan 24, 2017

Choose a reason for hiding this comment

domenic commented Jan 24, 2017

domenic commented Feb 8, 2017

ghost commented Mar 23, 2017 • edited by ghost Loading

zcorpan commented Mar 23, 2017

domenic commented Mar 23, 2017 • edited Loading

zcorpan commented Mar 23, 2017 • edited Loading

ghost commented Mar 23, 2017

Firefox (nightly channel)

All results

ghost commented Mar 23, 2017

Chrome (unstable channel)

All results

zcorpan commented Mar 23, 2017

ghost commented Mar 23, 2017

Firefox (nightly channel)

ghost commented Mar 23, 2017

Chrome (unstable channel)

domenic commented Jan 23, 2017 •

edited by gsnedders

Loading

wpt-stability-bot commented Jan 23, 2017 •

edited

Loading

wpt-stability-bot commented Jan 23, 2017 •

edited

Loading

ghost commented Mar 23, 2017 •

edited by ghost

Loading

domenic commented Mar 23, 2017 •

edited

Loading

zcorpan commented Mar 23, 2017 •

edited

Loading