-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Added a test for getClientRects().length. #6264
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
Added a test for getClientRects().length. #6264
Conversation
getClientRects() should be computed using CSS boxes, regardless of the source lines. This test verifies that having new lines in the source code does not change the number of rects the method returns.
|
This is for https://crbug.com/167261 which is a WebKit originated issue that still reproduces in Chrome and Safari. Edge and Firefox pass. |
Firefox (nightly)Testing web-platform-tests at revision b04b4a7 All results1 test ran/cssom-view/cssom-getClientRects-002.html
|
Sauce (safari)Testing web-platform-tests at revision 7ad7972 All results1 test ran/cssom-view/cssom-getClientRects-002.html
|
Chrome (unstable) |
Sauce (MicrosoftEdge)Testing web-platform-tests at revision b04b4a7 All results1 test ran/cssom-view/cssom-getClientRects-002.html
|
|
@phistuck can you please check the "allow edits from maintainers" checkbox in the sidebar, and comment when you have done so? |
|
@zcorpan - I am pretty sure I enabled it when I created the pull request... Weird. Perhaps a GitHub bug. |
|
Not the first time we're having problems with it being unchecked here. 😐 ok will push now |
| test(function () { | ||
| const count = document.querySelector("#single").getClientRects().length; | ||
| const count2 = document.querySelector("#multiple").getClientRects().length; | ||
| assert_equals(count, 1, "count"); |
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.
Did you intentionally remove the whole description of the test?
I mean, <title> still has it, but I thought it would be beneficial to understand what exactly it was trying to test when you look at the results...
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 test name is taken from the <title> if one isn't provided as an argument.
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 output in WebKit is
| Subtest | Results | Messages |
|---|---|---|
| OK | ||
CSSOM View - GetClientRects().length is the same regardless source new lines |
FAIL | assert_equals: count2 expected 1 but got 2 |
What count2 refers to should be clear when one looks at the source.
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.
Mmm cool.
Thank you.
|
Interesting.
Is this defined somewhee in the spec? I wasn't aware this was a requirement. Having a spec link in tests would be super helpful. |
| <html> | ||
| <head> | ||
| <title>CSSOM View - GetClientRects().length is the same regardless source new lines</title> | ||
| <link rel="help" href="https://drafts.csswg.org/cssom-view-1/#dom-element-getclientrects"> |
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.
@kojiishi - here is a specification link in the test.
|
@phistuck thank you, and sorry I misunderstood this was about |
|
On second thoughts, I'm not sure how this test is right again. So the spec for
From the linked spec of "box fragment", it reads to me that line boxes and inline boxes are also "box fragments", so they can produce a DOMRect. And as far as I understand, CSS does not define how to split DOM text nodes to inline boxes. Can you identify what I miss? |
|
I filed w3c/csswg-drafts#1545 for clarifying this, but I think both of us know that source lines are not very helpful here. |
getClientRects()should be computed using CSS boxes, regardless of the source lines.This test verifies that having new lines in the source code does not change the number of rects the method returns.