-
Notifications
You must be signed in to change notification settings - Fork 3.6k
add caretRangeFromPoint tests #54409
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 caretRangeFromPoint tests #54409
Conversation
mfreed7
left a comment
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.
Tests look ok, and despite the failures, I'm ok to land this. But I'd love to make sure the Blink failures are actual bugs, per the new spec PR.
css/cssom/caretRangeFromPoint-textarea-transform.tentative.html
Outdated
Show resolved
Hide resolved
| assert_true(range instanceof Range); | ||
| assert_equals(range.startOffset, 1); | ||
| assert_equals(range.endOffset, 1); | ||
| assert_equals(range.startContainer, textarea); |
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.
Blink returns the document as the container. Not sure if that's correct?
| assert_true(range instanceof Range); | ||
| assert_equals(range.startOffset, 0); | ||
| assert_equals(range.endOffset, 0); | ||
| assert_equals(range.startContainer, textarea); |
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.
Here and below, Blink returns <body>. Again, not sure what's correct.
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.
WebKit returns the <textarea> here. That makes sense to me since if a user clicked at (0, 0), the selection would be in the textarea.
| <script> | ||
| test(() => { | ||
| const textarea = document.createElement("textarea"); | ||
| document.replaceChild(textarea, document.documentElement); |
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.
In WebKit this line replaces the entire test content with <textarea></textarea> so nothing actually gets tested.
| assert_equals(range.startOffset, characterIndex); | ||
| assert_equals(range.endOffset, characterIndex); | ||
| assert_true(range.collapsed); | ||
| }, "document.caretRangeFromPoint() on a shadow should return a Range pointing at document.body"); |
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.
Why body and not the shadow host?
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.
The intent here is that it should match caretPositionFromPoint, so I've changed it to do that exactly.
This attempts to write some tests for w3c/csswg-drafts#12362
/cc @mfreed7 can I get a reviewer from the Chrome side please? Some of these tests will fail and impl may need adjusting as I think all browsers are inconsistent (
caretRangeFromPoint.tentative.htmlpasses but others don't - reporting the wrong node)./cc @annevk likewise - if I can get a suitable WebKit reviewer, as some of these will fail in WebKit.
/cc @gregorypappas