[Set Window Rect] Change parameter requirement from Number to Integer and range to "maximum safe integer"#1909
[Set Window Rect] Change parameter requirement from Number to Integer and range to "maximum safe integer"#1909yezhizhen wants to merge 2 commits intow3c:masterfrom
Conversation
index.html
Outdated
|
|
||
| <li><p>If <var>width</var> or <var>height</var> is neither null, nor | ||
| a <a>Number</a> from 0 to 2<sup>31</sup> − 1, return <a>error</a> | ||
| an integer from 0 to 2<sup>31</sup> − 1, return <a>error</a> |
There was a problem hiding this comment.
Good find. But lets take one more thing into account. In WebDriver BiDi we actually use a range from 0 to maximum safe integer. We probably should do the same here as well even though those high values won't be able to get applied. The same applies for x and y.
There was a problem hiding this comment.
In WebDriver BiDi we actually use a range from 0 to maximum safe integer.
Can you show a reference of this case in BiDi? I cannot find it anywhere.
I only see usage of "maximum safe integer" in Add Cookie/Timeout.
There was a problem hiding this comment.
The same applies for
xandy.
Also it seems x and y can be negative? How would we phrase it if upper bound is maximum safe integer?
There was a problem hiding this comment.
The CDDL definition uses https://w3c.github.io/webdriver-bidi/#cddl-type-js-uint. That's basically the maximum safe value. There is also js-int for the full range. In WebDriver classic see https://w3c.github.io/webdriver/#dfn-maximum-safe-integer.
yezhizhen
changed the title
|
web-platform-tests/wpt#53421 (comment) Can you also review this one after? Thanks!! |
|
I think this might actually be correct and BiDi wrong? It seems implausible that any implementation actually allows window dimensions larger than 32 bits (real life screen sizes typically fit into 13 bits). Also with high DPI screens it's possible that the width can be set to a non-integer number of CSS pixels (e.g. for a 2x DPI display a width of 800.5 CSS pixels would be 1601 device pixels). |
Yeah this is true. I also had similar doubt when seeing the test.. |
|
Do you think we should change the test instead of the spec? @jgraham |
|
I checked the test again. The set parameter can be floats. But response needs to be integer: (But also not explained why) Will open another issue and close the PR for now.. Because above test expectation is also confusing. |
|
I don't actually agree but lets discuss on the newly filed issue #1911. |
|
If we change the spec like this PR, I suppose we should also change above test to expect "invalid args" |
3 participants
The spec says Number, which is "double-precision 64-bit binary format IEEE 754 value". But the wpt test below clearly requires integer.
https://github.com/web-platform-tests/wpt/blob/b281d07f3ead48995b9a2e94259d292e22cd5dd9/webdriver/tests/classic/set_window_rect/set.py#L41-L49
This PR adds definition of minimum safe integer and change the valid range of parameter.
Preview | Diff