-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add leading/trailing whitespace subtests #49537
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file at the top has:
DO NOT EDIT! This test has been generated by /html/canvas/tools/gentest.py.
@@ -52,6 +58,7 @@ <h1>2d.text.measure.actualBoundingBox</h1> | |||
_assert(ctx.measureText('D').actualBoundingBoxDescent >= 12, "ctx.measureText('D').actualBoundingBoxDescent >= 12"); | |||
_assert(ctx.measureText('D').actualBoundingBoxDescent <= 15, "ctx.measureText('D').actualBoundingBoxDescent <= 15"); | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo?
_assert(ctx.measureText('A ').actualBoundingBoxRight >= 50, "ctx.measureText('A ').actualBoundingBoxRight >= 50"); | ||
|
||
_assert(Math.abs(ctx.measureText(' A').actualBoundingBoxLeft) >= 50, "Math.abs(ctx.measureText(' A').actualBoundingBoxLeft) >= 50"); | ||
_assert(ctx.measureText(' A').actualBoundingBoxRight <= 101, "ctx.measureText(' A').actualBoundingBoxRight <= 101"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be useful to separate this out so it can be tested standalone. I think we also want to test some other whitespace code points. Unicode has a lot of them.
Oops still not enough CAPS for me clearly :) Will fix. |
Add leading/trailing whitespace subtests to verify the effects on actualBoundingBoxLeft/actualBoundingBoxRight text metrics.
f8c2bae
to
995fe84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a good start, but I would really recommend adding tests for more types of whitespace to ensure implementations are aligned on what whitespace gets ignored. See https://en.wikipedia.org/wiki/Whitespace_character for inspiration.
Add leading/trailing whitespace subtests to verify the effects on actualBoundingBoxLeft/actualBoundingBoxRight text metrics.