Closed
Bug 1482042
Opened 6 years ago
Closed 6 years ago
[wdspec] Make "test_negative_x_y" and "test_height_width" independent from system settings
Categories
(Testing :: geckodriver, enhancement, P1)
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
jgraham
:
review+
|
Details | Diff | Splinter Review |
Both tests are failing for me locally, and I noticed that when working on the Serde refactoring (bug 1396821).
1) "test_negative_x_y" makes the assumption that the menu bar on OS X has always a height of 21px. This is not true for my system, where it is 22px height. To make this test work on all systems we have to make use of `window.screen.availTop`.
2) "test_height_width" fails only in combination with other tests of the same file. And that only if the original position causes the window to be moved by the browser to fit in screen when it's new size would enlarge it. To fix this the position should be set to `(50, 50)` while the height and width is 100px smaller than the available size.
I would like to see those tests passing fine for my work on bug 1396821, so I mark it as blocking.
Assignee | ||
Updated•6 years ago
|
Summary: [wdspec] Make "test_negative_x_y" and "test_height_width" independent from system dependencies → [wdspec] Make "test_negative_x_y" and "test_height_width" independent from system settings
Assignee | ||
Comment 1•6 years ago
|
||
Both tests are failing due to differnt system settings:
"test_negative_x_y" makes the assumption that the menu
bar on MacOS has always a height of 21px. This is not
true for all systems, because it can vary due to system
settings (font, locale). To make this test work correctly
on all systems the minimum y position can be retrieved
with "window.screen.availTop`.
"test_height_width" fails in combination with other test,
and that only if the window get moved by the browser to
fit into the screen when a resize would enlarge it outside
of the screen. To fix it a known to work start position
has to be set.
MozReview-Commit-ID: Eu2EN0XT39x
Assignee | ||
Comment 2•6 years ago
|
||
Both tests are failing due to differnt system settings:
"test_negative_x_y" makes the assumption that the menu
bar on MacOS has always a height of 21px. This is not
true for all systems, because it can vary due to system
settings (font, locale). To make this test work correctly
on all systems the minimum y position can be retrieved
with "window.screen.availTop`.
"test_height_width" fails in combination with other test,
and that only if the window get moved by the browser to
fit into the screen when a resize would enlarge it outside
of the screen. To fix it a known to work start position
has to be set.
MozReview-Commit-ID: Eu2EN0XT39x
Assignee | ||
Updated•6 years ago
|
Attachment #8998742 -
Attachment is obsolete: true
Assignee | ||
Comment 3•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #8998743 -
Flags: review?(james)
Comment 4•6 years ago
|
||
Comment on attachment 8998743 [details] [diff] [review]
[wdspec] Make "test_negative_x_y" and "test_height_width" independent from system settings
Review of attachment 8998743 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/web-platform/tests/webdriver/tests/set_window_rect/set.py
@@ +168,5 @@
>
>
> def test_height_width(session):
> + # The window position might be auto-adjusted by the browser
> + # if it exceeds the lower right corner. As such ensure that
I don't really know what it mean to excced a corner. I guess you mean that the position might be adjusted if the window bounds are outside the bounds of the desktop?
@@ +256,5 @@
> # being moved to (0,0).
> elif os == "mac":
> + value = assert_success(response)
> +
> + avail_top = session.execute_script("return window.screen.availTop;")
Unfortunately this seems to be nonstandard, so it's tough to justify relying on it here. CSSOM View only seems to have screen.height and screen.avilHeight, and I guess on OSX the dock also takes up some of the difference? I'm not sure what the best solution is tbh; we might just need to convert the condition to `y <= screen.height - screen.avilHeight` or something.
Attachment #8998743 -
Flags: review?(james) → review-
Assignee | ||
Comment 5•6 years ago
|
||
Comment on attachment 8998743 [details] [diff] [review]
[wdspec] Make "test_negative_x_y" and "test_height_width" independent from system settings
Review of attachment 8998743 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/web-platform/tests/webdriver/tests/set_window_rect/set.py
@@ +168,5 @@
>
>
> def test_height_width(session):
> + # The window position might be auto-adjusted by the browser
> + # if it exceeds the lower right corner. As such ensure that
Correct. If the window doens't fit into screen at least Firefox moves the window.
@@ +256,5 @@
> # being moved to (0,0).
> elif os == "mac":
> + value = assert_success(response)
> +
> + avail_top = session.execute_script("return window.screen.availTop;")
From the browser compatibility section on MDN (https://developer.mozilla.org/en-US/docs/Web/API/Screen#Browser_compatibility) all browsers should have this property implemented, except IE and Edge. But both do not run on MacOS. So shouldn't we be fine here?
Also I tested with latest Chrome, and Safari, and can verify that both have `availTop` set.
Assignee | ||
Comment 6•6 years ago
|
||
CC'ing Jim for additional input in regards of `window.screen.availTop`.
Comment 7•6 years ago
|
||
Given it's nonstandard, I think you at the very least need a comment explaining that it's not standard but exists in the implemenatations we carea about, and a fallback path that uses the CSSOM View properties even if the test has to be weaker in that case.
I'm fine using `availTop` on macOS only. It's true that IE and Edge don't implement `window.screen.availTop`, but given that neither browser runs on anything other than Windows, that seems like a fine approach.
Assignee | ||
Comment 9•6 years ago
|
||
Both tests are failing due to differnt system settings:
"test_negative_x_y" makes the assumption that the menu
bar on MacOS has always a height of 21px. This is not
true for all systems, because it can vary due to system
settings (font, locale). To make this test work correctly
on all systems the minimum y position can be retrieved
with "window.screen.availTop`. While this CSSOM interface
is not standardized, all of the browsers we care about on
MacOS have it implemented.
"test_height_width" fails in combination with other test,
and that only if the window get moved by the browser to
fit into the screen when a resize would enlarge it outside
of the screen. To fix it a known to work start position
has to be set.
MozReview-Commit-ID: Eu2EN0XT39x
Assignee | ||
Updated•6 years ago
|
Attachment #8998743 -
Attachment is obsolete: true
Assignee | ||
Comment 10•6 years ago
|
||
Both tests are failing due to differnt system settings:
"test_negative_x_y" makes the assumption that the menu
bar on MacOS has always a height of 21px. This is not
true for all systems, because it can vary due to system
settings (font, locale). To make this test work correctly
on all systems the minimum y position can be retrieved
with "window.screen.availTop`. While this CSSOM interface
is not standardized, all of the browsers we care about on
MacOS have it implemented.
"test_height_width" fails in combination with other test,
and that only if the window get moved by the browser to
fit into the screen when a resize would enlarge it outside
of the screen. To fix it a known to work start position
has to be set.
MozReview-Commit-ID: Eu2EN0XT39x
Assignee | ||
Updated•6 years ago
|
Attachment #8999853 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8999855 -
Flags: review?(james)
Comment 11•6 years ago
|
||
Comment on attachment 8999855 [details] [diff] [review]
[wdspec] Make "test_negative_x_y" and "test_height_width" independent from system settings
Review of attachment 8999855 [details] [diff] [review]:
-----------------------------------------------------------------
I still think this should fall back to something more standard, but OK.
Attachment #8999855 -
Flags: review?(james) → review+
Assignee | ||
Comment 12•6 years ago
|
||
(In reply to James Graham [:jgraham] from comment #11)
> I still think this should fall back to something more standard, but OK.
We won't be able to accurately test this anymore. It would only be doable via a range of some pixels. I'm happy to implement this in a follow-up bug if it turns out to be a problem in some cases.
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 13•6 years ago
|
||
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/83793017b086
[wdspec] Make "test_negative_x_y" and "test_height_width" independent from system settings. r=jgraham
Keywords: checkin-needed
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12479 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Comment 16•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Upstream PR merged
You need to log in
before you can comment on or make changes to this bug.
Description
•