Trigger WindowEvent::Resize when resizing in headless mode #15818

alanhoff · 2017-03-04T05:52:26Z

When resizing in headless mode a resize event is now fired

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #15621

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2017-03-04T05:52:32Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @SimonSapin (or someone else) soon.

alanhoff · 2017-03-04T05:53:45Z

@jdm I believe this code fixes #15621, now I need some help on how I should add tests.

jdm · 2017-03-04T06:34:12Z

If you follow these instructions, you can add a new test in tests/wpt/mozilla/tests/mozilla using the testharness.js API (you probably want to use async_test).


        Trigger WindowEvent::Resize when resizing in headless mode


        Implement WPT tests for windows.onresize

alanhoff · 2017-03-04T15:53:52Z

@jdm @SimonSapin ready for review

jdm · 2017-03-05T01:11:24Z

We should add an assert that window.clientWidth and clientHeight equal the new dimension inside of the event handler, too.

alanhoff · 2017-03-05T05:49:39Z

@jdm I just submitted the tests you asked for. Everything is working correctly :-)

jdm

This ends up testing that clientWidth == clientHeight, and uses a descriptive label of "200". We should use two separate uses of assert_equals instead.

alanhoff · 2017-03-05T15:24:20Z

@jdm oh that was some ugly mistake – by reading the examples I thought that assert_equals would compare equality across all arguments.

So, it turns out that both clientWidth and clientHeight are undefined and that makes me think that they're not being set at all during the window's lifecycle. Having said that I think it's a separated issue (one that I'm also interested in solving) that deserves an equally separated PR. What do you think?

Also, do you have ideias why this build is failing?

jdm · 2017-03-06T03:34:37Z

Oops, I meant innerWidth/innerHeight; clientWidth/clientHeight is a property of Element objects, not Window.

jdm · 2017-03-06T03:36:15Z

The travis build is failing because you have modified a test file without updating the manifest (I know, it's rather annoying). You will need to run ./mach update-manifest every time and commit the changes to MANIFEST.json before pushing after modifying the test file.


        Ensure innerWidth and innerHeight equals new dimensions

alanhoff · 2017-03-06T04:12:58Z

@jdm Now it makes sense and tests are passing, thx for the tips.
Ready for review

alanhoff · 2017-03-12T23:18:26Z

@jdm any updates about this pr? :-)

jdm · 2017-03-13T13:28:17Z

Sorry, I've been on vacation. I will look at my review queue (including this PR) starting Wednesday.

nox · 2017-03-14T09:09:38Z

@bors-servo try

bors-servo · 2017-03-14T09:09:42Z

⌛ Trying commit 3bf1ede with merge 5127faa...


        Auto merge of #15818 - alanhoff:resize-event-headless, r=<try>

Trigger WindowEvent::Resize when resizing in headless mode When resizing in headless mode a resize event is now fired ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15621  - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15818)

Single remark addressed by PR author.

bors-servo · 2017-03-14T09:34:42Z

💔 Test failed - linux-rel-wpt

nox · 2017-03-14T10:37:36Z

  ▶ TIMEOUT [expected OK] /_mozilla/mozilla/resizeTo.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  └ 3.3 (Core Profile) Mesa 12.0.1

  ▶ Unexpected subtest result in /_mozilla/mozilla/resizeTo.html:
  │ TIMEOUT [expected PASS] onresize
  └   → Test timed out

alanhoff · 2017-03-14T16:18:56Z

@jdm @nox I need guidance on what went wrong with the homu test and how to fix it

KiChjang · 2017-03-14T17:37:31Z

@alanhoff Are you able to reproduce the test failures locally on your machine?

jdm · 2017-03-15T04:01:24Z

Does the test pass locally with --no-pause-after-test? That flag ensures that the test is run in a headless environment.

alanhoff · 2017-03-16T02:18:25Z

@jdm it completes without error

./mach test tests/wpt/mozilla/tests/mozilla/resizeTo.html --no-pause-after-test
Running 1 tests in web-platform-tests

Ran 1 tests finished in 53.0 seconds.
  • 1 ran as expected. 0 tests skipped.

Tests completed in 57.49s

alanhoff · 2017-03-16T02:19:46Z

@KiChjang I don't know exactly the commend that the test runner used to yield that error, could you help me with that?

jdm · 2017-03-16T12:09:00Z

I'm not sure why the test take almost 60s for you, but when I run ./mach test-wpt tests/wpt/mozilla/tests/mozilla/resizeTo.html --no-pause-after-test locally it takes about 5s for me. I'm really confused about why the build machines are timing out :<

bors-servo · 2017-06-19T21:42:09Z

☔ The latest upstream changes (presumably #17410) made this pull request unmergeable. Please resolve the merge conflicts.

nox · 2017-10-07T16:05:05Z

@alanhoff Do you need help with this?

jdm · 2017-10-08T12:25:16Z

@nox I need help figuring out why the test works locally and times out on the build machines.

dralley · 2020-04-14T22:10:00Z

Since Glutin is going away and the issues with this PR were never fixed, it can be closed :(

This is still a problem however, even with the Surfman + Winit PR branch, so the issue should stay open.

highfive assigned SimonSapin Mar 4, 2017

highfive added the S-awaiting-review label Mar 4, 2017

alanhoff force-pushed the alanhoff:resize-event-headless branch from dcaffbe to 9e8ff67 Mar 4, 2017

Trigger WindowEvent::Resize when resizing in headless mode

Loading status checks…

8b19cd5

alanhoff force-pushed the alanhoff:resize-event-headless branch from 9e8ff67 to 8b19cd5 Mar 4, 2017

Implement WPT tests for windows.onresize

Loading status checks…

bafdbbd

alanhoff force-pushed the alanhoff:resize-event-headless branch from cedb71d to bafdbbd Mar 4, 2017

alanhoff changed the title ~~[wip] Trigger WindowEvent::Resize when resizing in headless mode~~ Trigger WindowEvent::Resize when resizing in headless mode Mar 4, 2017

jdm reviewed Mar 5, 2017

View changes

Ensure innerWidth and innerHeight equals new dimensions

Loading status checks…

3bf1ede

alanhoff force-pushed the alanhoff:resize-event-headless branch from 581fbe5 to 3bf1ede Mar 6, 2017

SimonSapin assigned jdm and unassigned SimonSapin Mar 13, 2017

nox assigned nox and unassigned jdm Mar 14, 2017

highfive added the S-tests-failed label Mar 14, 2017

nox removed the S-awaiting-review label Mar 15, 2017

highfive added the S-needs-rebase label Jun 19, 2017

jdm closed this Apr 14, 2020

servo / servo

Trigger WindowEvent::Resize when resizing in headless mode #15818

Trigger WindowEvent::Resize when resizing in headless mode #15818

alanhoff commented Mar 4, 2017 •

edited

highfive commented Mar 4, 2017

alanhoff commented Mar 4, 2017

jdm commented Mar 4, 2017

alanhoff commented Mar 4, 2017

jdm commented Mar 5, 2017

alanhoff commented Mar 5, 2017

jdm left a comment

alanhoff commented Mar 5, 2017

jdm commented Mar 6, 2017

jdm commented Mar 6, 2017

alanhoff commented Mar 6, 2017

alanhoff commented Mar 12, 2017

jdm commented Mar 13, 2017

nox commented Mar 14, 2017

bors-servo commented Mar 14, 2017

bors-servo commented Mar 14, 2017

nox commented Mar 14, 2017

alanhoff commented Mar 14, 2017

KiChjang commented Mar 14, 2017

jdm commented Mar 15, 2017

alanhoff commented Mar 16, 2017

alanhoff commented Mar 16, 2017

jdm commented Mar 16, 2017 •

edited

bors-servo commented Jun 19, 2017

nox commented Oct 7, 2017

jdm commented Oct 8, 2017

dralley commented Apr 14, 2020

servo / servo

Join GitHub today

Trigger WindowEvent::Resize when resizing in headless mode #15818

Trigger WindowEvent::Resize when resizing in headless mode #15818

Conversation

alanhoff commented Mar 4, 2017 • edited

highfive commented Mar 4, 2017

alanhoff commented Mar 4, 2017

jdm commented Mar 4, 2017

alanhoff commented Mar 4, 2017

jdm commented Mar 5, 2017

alanhoff commented Mar 5, 2017

jdm left a comment

alanhoff commented Mar 5, 2017

jdm commented Mar 6, 2017

jdm commented Mar 6, 2017

alanhoff commented Mar 6, 2017

alanhoff commented Mar 12, 2017

jdm commented Mar 13, 2017

nox commented Mar 14, 2017

bors-servo commented Mar 14, 2017

bors-servo commented Mar 14, 2017

nox commented Mar 14, 2017

alanhoff commented Mar 14, 2017

KiChjang commented Mar 14, 2017

jdm commented Mar 15, 2017

alanhoff commented Mar 16, 2017

alanhoff commented Mar 16, 2017

jdm commented Mar 16, 2017 • edited

bors-servo commented Jun 19, 2017

nox commented Oct 7, 2017

jdm commented Oct 8, 2017

dralley commented Apr 14, 2020

alanhoff commented Mar 4, 2017 •

edited

jdm commented Mar 16, 2017 •

edited