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

Tweak the useWorkerFetch default value checks (PR 15879 follow-up) #16758

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Jul 27, 2023

Currently we accidentally accept cMapUrl and standardFontDataUrl parameters that are empty strings or null, since e.g. new URL(null, document.baseURI) doesn't throw, when validating the useWorkerFetch parameter via the isValidFetchUrl helper function.
Please note that we are currently failing gracefully in this case, as intended, however the warning-messages printed in the console are perhaps less helpful without this patch.

@Snuffleupagus Snuffleupagus changed the title Tweak the useWorkerFetch default value checks (PR 15879) Tweak the useWorkerFetch default value checks (PR 15879 follow-up) Jul 27, 2023
@Snuffleupagus Snuffleupagus force-pushed the cMapUrl-standardFontDataUrl-validation branch from 86c8a63 to 0154f2f Compare July 27, 2023 14:25
Currently we accidentally accept `cMapUrl` and `standardFontDataUrl` parameters that are empty strings or `null`, since e.g. `new URL(null, document.baseURI)` doesn't throw, when validating the `useWorkerFetch` parameter via the `isValidFetchUrl` helper function.
Please note that we are currently failing gracefully in this case, as intended, however the warning-messages printed in the console are perhaps less helpful without this patch.
@Snuffleupagus Snuffleupagus force-pushed the cMapUrl-standardFontDataUrl-validation branch from 0154f2f to c09bd55 Compare July 27, 2023 14:27
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.241.84.105:8877/8e72ac68c1ee778/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.193.163.58:8877/ab83064a4720425/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/8e72ac68c1ee778/output.txt

Total script time: 26.29 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 19
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/8e72ac68c1ee778/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/ab83064a4720425/output.txt

Total script time: 38.16 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 26

Image differences available at: http://54.193.163.58:8877/ab83064a4720425/reftest-analyzer.html#web=eq.log

Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@Snuffleupagus Snuffleupagus merged commit 5560643 into mozilla:master Jul 27, 2023
3 checks passed
@Snuffleupagus Snuffleupagus deleted the cMapUrl-standardFontDataUrl-validation branch July 27, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants