Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safari "stable" consistently crashing on new pointerevents test #17567

Closed
jugglinmike opened this issue Jun 28, 2019 · 27 comments
Closed

Safari "stable" consistently crashing on new pointerevents test #17567

jugglinmike opened this issue Jun 28, 2019 · 27 comments

Comments

@jugglinmike
Copy link
Contributor

Safari 12.1 has failed to upload results for /pointerevents/pointerlock/pointerevent_movementxy.html since the introduction of that test on 2019-06-20.

https://wpt.fyi/results/pointerevents/pointerlock?sha=9c6304b&label=master&product=chrome%5Bexperimental%5D&product=edge%5Bexperimental%5D&product=firefox%5Bexperimental%5D&product=safari%5Bstable%5D

I observed this from the results collection project via the following WebDriver error:

WebDriverException: not implemented (501): Input sources with a pointerType of 'touch' are not currently supported.

but I have not yet researched the Azure Pipelines logs produced by the corresponding collection attempt from this project.

@foolip
Copy link
Member

foolip commented Jul 8, 2019

@burg @grorg @youennf do you who might be able to take a look at this?

@eric-carlson
Copy link

@graouts may be interested.

@graouts
Copy link
Contributor

graouts commented Jul 8, 2019

Hi! I’ve been writing the WebKit Pointer Events implementation. How can i get set up to run the program that encounters this issue?

@foolip
Copy link
Member

foolip commented Jul 18, 2019

@jugglinmike do you have the command line handy for what reproduces this?

@foolip
Copy link
Member

foolip commented Jul 25, 2019

I don't believe this is urgent, but something we should resolve. Assigning to Mike to give repro steps for @graouts.

@jugglinmike
Copy link
Contributor Author

Hi, @graouts! I can consistently reproduce this on a Mac Mini running High Sierra and Safari 12.

macOS: 10.13.6
Safari: Version 12.0.1 (13606.2.100)

https://web-platform-tests.org should have all the information necessary to get WPT running on macOS (if it doesn't, I'll be happy to help you and extend the docs).

Once that's set up, the following command should demonstrate the problem:

$ ./wpt run --no-pause safari pointerevents/pointerlock/pointerevent_movementxy.html 
Here's the output I observe on my system
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
You are using pip version 19.0.3, however version 19.2.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
You are using pip version 19.0.3, however version 19.2.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
safaridriver: unrecognized option `--version'
Running 1 tests in web-platform-tests

  ▶ ERROR [expected OK] /pointerevents/pointerlock/pointerevent_movementxy.html
  └   → Input sources with a pointerType of 'touch' are not currently supported.
Traceback (most recent call last):
  File "/Users/kazooie/wpt/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 286, in _run
    self.result = True, self.func(self.protocol, self.url, self.timeout)
  File "/Users/kazooie/wpt/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 377, in do_testharness
    done, rv = handler(result)
  File "/Users/kazooie/wpt/tools/wptrunner/wptrunner/executors/base.py", line 604, in __call__
    return callback(url, payload)
  File "/Users/kazooie/wpt/tools/wptrunner/wptrunner/executors/base.py", line 618, in process_action
    action_handler(payload)
  File "/Users/kazooie/wpt/tools/wptrunner/wptrunner/executors/base.py", line 673, in __call__
    self.protocol.action_sequence.send_actions({"actions": actions})
  File "/Users/kazooie/wpt/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 179, in send_actions
    self.webdriver.actions.perform(actions['actions'])
  File "/Users/kazooie/wpt/tools/webdriver/webdriver/client.py", line 20, in inner
    return func(self, *args, **kwargs)
  File "/Users/kazooie/wpt/tools/webdriver/webdriver/client.py", line 221, in perform
    actions = self.session.send_session_command("POST", "actions", body)
  File "/Users/kazooie/wpt/tools/webdriver/webdriver/client.py", line 516, in send_session_command
    return self.send_command(method, url, body)
  File "/Users/kazooie/wpt/tools/webdriver/webdriver/client.py", line 480, in send_command
    raise err
WebDriverException: not implemented (501): Input sources with a pointerType of 'touch' are not currently supported.



Ran 1 tests finished in 1.7 seconds.
  • 0 ran as expected. 0 tests skipped.
  • 1 tests had errors unexpectedly

Thanks for helping us look in to this!

@graouts
Copy link
Contributor

graouts commented Aug 13, 2019

Cc'ing @burg who might know something about the exception raised here.

@burg
Copy link
Contributor

burg commented Aug 14, 2019

I can reproduce the hang locally on Catalina + Safari 13, investigating now.

@burg
Copy link
Contributor

burg commented Aug 15, 2019

It’s been fixed in WebKit trunk. I don’t know which Safari Technology Preview release will include the change.

@graouts
Copy link
Contributor

graouts commented Aug 19, 2019

I believe the fix for this was https://trac.webkit.org/changeset/248696.

@foolip
Copy link
Member

foolip commented Aug 19, 2019

I suppose the only way to confirm the fix will be to see the results in a new version of STP, so resolving this is blocked on #18461.

@jugglinmike
Copy link
Contributor Author

Thanks, @burg!

@graouts
Copy link
Contributor

graouts commented Aug 21, 2019

Safari Technology Preview 90 should have this fix (assuming I have the right revision number above).

@foolip
Copy link
Member

foolip commented Aug 22, 2019

@graouts
Copy link
Contributor

graouts commented Aug 22, 2019

The WebKit bug 174030 has a patch up for it and I've pinged the best person to review it. Hopefully it'll land shortly and make it into the next STP build.

Meanwhile, should wait until a new STP build has been adopted by WPT.fyi to resolve this issue or can it be closed now?

@foolip
Copy link
Member

foolip commented Aug 22, 2019

The issue was filed about Safari stable, which we'd have to wait a very long time to see resolved, but I think we can close this when we have a run on the front page of wpt.fyi from STP >=90 proving this has been resolved.

@graouts
Copy link
Contributor

graouts commented Sep 18, 2019

The WebKit bug 174030 has been resolved and STP 92 contains its fix. Is there anything else blocking this on the WebKit side?

@foolip
Copy link
Member

foolip commented Sep 18, 2019

I'm trying in #19142 to upgrade to STP 92, but as I wrote there I have doubts if the fix is included there. I'll comment on that PR what's happening, anyone who can help, please jump in.

@foolip
Copy link
Member

foolip commented Oct 3, 2019

Current results:
https://wpt.fyi/results/pointerevents?run_id=316730006&run_id=343870004

I think this is a matter of a specific feature not working, possibly a problem in safaridriver, but not affecting overall results, so I'll remove this from https://github.com/web-platform-tests/wpt/projects/4 again.

@graouts
Copy link
Contributor

graouts commented Oct 14, 2019

Current results:
https://wpt.fyi/results/pointerevents?run_id=316730006&run_id=343870004

When I follow this link STP 82 is still shown. Is that expected? I thought this would somehow show a recent STP.

@foolip
Copy link
Member

foolip commented Oct 14, 2019

@graouts see #17186, upgrading to a more recent version of STP has been blocked on a long series of issues, currently sudo safaridriver --enable not working.

More details in my failed attempts #18925, #19142 and #19482. Hopefully STP 94 will be possible to install and run in a CI system again, but I don't know when it will be released.

@foolip
Copy link
Member

foolip commented Oct 14, 2019

Since this was about Safari stable, these diffs between Safari 12.1 and 13 might be interesting:
https://wpt.fyi/results/?diff&filter=ADC&run_id=311390003&run_id=330280040
https://wpt.fyi/results/?diff&filter=ADC&run_id=307790006&run_id=342180031

(Either will do, they should differ only due to flakiness.)

The tests in pointerevents/ seem to be doing much better now. The pointerevent_movementxy.html test that this was reported against is now failing in a different way, not a harness error.

There are still a lot of tests that fail with "WebDriverException: not implemented" however, searching for status:error on wpt.fyi reveals them.

@stephenmcgruer
Copy link
Contributor

Looking at our .azure-pipelines.yml, we still have a lot of tests marked as excluded, though each seem to have their own issue filed:

  # --exclude is a workaround for https://github.com/web-platform-tests/wpt/issues/18995 + https://github.com/web-platform-tests/wpt/issues/20887 + https://github.com/web-platform-tests/wpt/issues/22175 + https://github.com/web-platform-tests/wpt/issues/23630
  - script: no_proxy='*' ./wpt run --no-manifest-update --no-restart-on-unexpected --no-fail-on-unexpected --this-chunk=$(System.JobPositionInPhase) --total-chunks=$(System.TotalJobsInPhase) --chunk-type hash --log-wptreport $(Build.ArtifactStagingDirectory)/wpt_report_$(System.JobPositionInPhase).json --log-wptscreenshot $(Build.ArtifactStagingDirectory)/wpt_screenshot_$(System.JobPositionInPhase).txt --log-mach - --log-mach-level info safari --exclude /pointerevents/pointerevent_pointercapture-not-lost-in-chorded-buttons.html --exclude /pointerevents/pointerevent_pointercapture_in_frame.html --exclude /web-share/share-sharePromise-internal-slot.https.html --exclude /webdriver/tests/perform_actions/pointer_tripleclick.py

This includes two pointerevents tests, which (according to wpt.fyi) pass on Safari Tech Preview 106. We obviously don't know their results on Safari Stable because they are excluded.

Based on 5edc008, /pointerevents/pointerevent_pointercapture_in_frame.html crashing is new at least as of 13.1.

This issue feels like we're tracking the wrong thing though - it's bad that Safari stable crashes for these tests but that should presumably be a WebKit bug, not a WPT issue. On the WPT side, surely we should care about the fact that we can't handle this sort of crash instead? (Which appears to be #19620 I believe).

@foolip
Copy link
Member

foolip commented May 25, 2020

Yes, that's #19620. If we handled crashes, then we wouldn't need to add --exclude to recover from new breakage.

These are all WPT issues because it's been urgent in every case to fix it. I think that someone one the WebKit team has been made aware in each case though, but didn't double check now.

@stephenmcgruer
Copy link
Contributor

These are all WPT issues because it's been urgent in every case to fix it. I think that someone one the WebKit team has been made aware in each case though, but didn't double check now.

Right, but assuming we filed a WebKit bug for each (did we?), the idea would be to close the WPT issue once the problem is not causing pain on our end (currently when we add --exclude, but in a bright future they'd just never cause us pain because we could actually handle Safari crashing in the CI).

@foolip
Copy link
Member

foolip commented May 25, 2020

assuming we filed a WebKit bug for each (did we?)

Check each bug still linked in .azure-pipelines.yml:

So not as good as I thought. Perhaps the ones that we haven't filed any bugs for are exactly the issues that remain unfixed.

This issue can be closed, though, it's not listed in .azure-pipelines.yml.

@foolip foolip closed this as completed May 25, 2020
@graouts
Copy link
Contributor

graouts commented Jun 4, 2020

Please file bugs with repro steps on bugs.webkit.org and I'll be sure to investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants