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

[MotionMark] Live version of 4 tests just show a 403 Access Denied page #12213

Conversation

litherum
Copy link
Contributor

@litherum litherum commented Mar 30, 2023

7e8331b

[MotionMark] Live version of 4 tests just show a 403 Access Denied page
https://bugs.webkit.org/show_bug.cgi?id=254782
rdar://107208247

Reviewed by Alexey Proskuryakov.

Apparently, the firewall that the live MotionMark site is behind doesn't like it when paths that include
the "../" string are part of query parameters in URLs. Luckily, MotionMark is already set up to do the right
thing when the query parameter is omitted, so this patch just deletes those query parameters and uses the
test-specific logic to fall back to the correct string anyway.

* Websites/browserbench.org/MotionMark1.2/resources/debug-runner/tests.js:
* Websites/browserbench.org/MotionMark1.2/tests/bouncing-particles/resources/bouncing-canvas-images.js:
(BouncingCanvasParticlesStage.call.initialize):
* Websites/browserbench.org/MotionMark1.2/tests/bouncing-particles/resources/bouncing-svg-images.js:
(BouncingSvgParticlesStage.call.initialize):

Canonical link: https://commits.webkit.org/262382@main

cb4340e

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac   πŸ›  wpe   πŸ›  wincairo
βœ… πŸ›  ios-sim   πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac   πŸ›  gtk
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
  πŸ›  tv βœ… πŸ§ͺ mac-wk2   πŸ§ͺ api-gtk
  πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
  πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@litherum litherum self-assigned this Mar 30, 2023
@litherum litherum added the Layout and Rendering For bugs with layout and rendering of Web pages. label Mar 30, 2023
@litherum litherum force-pushed the eng/MotionMark-Live-version-of-4-tests-just-show-a-403-Access-Denied-page branch from b47aa4c to cb4340e Compare March 30, 2023 23:48
@@ -49,7 +49,7 @@ BouncingCanvasImagesStage = Utilities.createSubclass(BouncingCanvasParticlesStag
initialize: function(benchmark, options)
{
BouncingCanvasParticlesStage.prototype.initialize.call(this, benchmark, options);
var imageSrc = options["imageSrc"] || "resources/yin-yang.svg";
var imageSrc = options["imageSrc"] || "../resources/yin-yang.svg";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually work? Because there's still a ../. Or is that OK because it's not in query any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does work. I've worked with Ling to run a test version of the same server, and verified that it does, in fact, fix the problem. The problem doesn't seem to be relative paths themselves, but just when ../ paths appear inside query parameters.

@litherum litherum added the merge-queue Applied to send a pull request to merge-queue label Mar 30, 2023
https://bugs.webkit.org/show_bug.cgi?id=254782
rdar://107208247

Reviewed by Alexey Proskuryakov.

Apparently, the firewall that the live MotionMark site is behind doesn't like it when paths that include
the "../" string are part of query parameters in URLs. Luckily, MotionMark is already set up to do the right
thing when the query parameter is omitted, so this patch just deletes those query parameters and uses the
test-specific logic to fall back to the correct string anyway.

* Websites/browserbench.org/MotionMark1.2/resources/debug-runner/tests.js:
* Websites/browserbench.org/MotionMark1.2/tests/bouncing-particles/resources/bouncing-canvas-images.js:
(BouncingCanvasParticlesStage.call.initialize):
* Websites/browserbench.org/MotionMark1.2/tests/bouncing-particles/resources/bouncing-svg-images.js:
(BouncingSvgParticlesStage.call.initialize):

Canonical link: https://commits.webkit.org/262382@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/MotionMark-Live-version-of-4-tests-just-show-a-403-Access-Denied-page branch from cb4340e to 7e8331b Compare March 31, 2023 00:42
@webkit-commit-queue
Copy link
Collaborator

Committed 262382@main (7e8331b): https://commits.webkit.org/262382@main

Reviewed commits have been landed. Closing PR #12213 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 7e8331b into WebKit:main Mar 31, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 31, 2023
@litherum litherum deleted the eng/MotionMark-Live-version-of-4-tests-just-show-a-403-Access-Denied-page branch March 31, 2023 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
4 participants