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)

Version 3
enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
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
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
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
Attachment #8998742 - Attachment is obsolete: true
Attachment #8998743 - Flags: review?(james)
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-
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.
CC'ing Jim for additional input in regards of `window.screen.availTop`.
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.
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
Attachment #8998743 - Attachment is obsolete: true
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
Attachment #8999853 - Attachment is obsolete: true
Attachment #8999855 - Flags: review?(james)
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+
(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.
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.
https://hg.mozilla.org/mozilla-central/rev/83793017b086
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: