Closed Bug 1340775 Opened 8 years ago Closed 8 years ago

Intermittent test_window_position.py TestWindowPosition.test_move_to_existing_position | InvalidArgumentException: Expected [object Number] -8 to be >= 0

Categories

(Testing :: Marionette Client and Harness, defect)

Product:
Testing
Component:
Marionette Client and Harness
Version:
Version 3
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox53 fixed, firefox54 fixed, firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: ato)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Bug 1340775 - Allow window position to be set to negative coordinates;
59 bytes, text/x-review-board-request
impossibus
: review+
Details
Flags: needinfo?(VYV03354)
window.screenX will be -8 if the window is maximized. The window is maximized somehow?
Flags: needinfo?(VYV03354)
That is correct. If a window is maximised on Windows, it will sit -8 pixels off the main screen. I’ve had this confirmed by Windows users before, but since I don’t use Windows I don’t actually know how that works.
jimm: When Firefox is maximised on Windows, window.screenX reports -8 px. Is this expected behaviour or a bug on our part? It appears that Blink reports 0 px in the same situation, which leads me to think there is an interop issue here. What are the expected values of window.screenX/window.screenY when a browser is maximised?
Flags: needinfo?(jmathies)
In any case: In the context of the WebDriver API, it should be possible to move a window off the screen. The positive integer bounds check in GeckoDriver.prototype.setWindowPosition@chrome://marionette/content/driver.js:1184:3 is consequently incorrect and I’ve filed https://github.com/w3c/webdriver/issues/804 on the specification.
Submitted https://github.com/w3c/webdriver/pull/805 to the WebDriver specification to allow x/y fields to be signed integers.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Blocks: webdriver
Comment on attachment 8840426 [details] Bug 1340775 - Allow window position to be set to negative coordinates; https://reviewboard.mozilla.org/r/114938/#review116740 r+wc ::: testing/marionette/harness/marionette_harness/tests/unit/test_window_position.py:52 (Diff revision 2) > + print("Current position: {}".format( > + self.marionette.get_window_position())) > + self.marionette.set_window_position(-8, -8) > + print("Position after requesting move to negative coordinates: {}".format( > + self.marionette.get_window_position())) > + You could assert that the coordinates are in some rough range, so that we're more likely to notice if something unusual is happening. Up to you.
Attachment #8840426 - Flags: review?(mjzffr) → review+
Comment on attachment 8840426 [details] Bug 1340775 - Allow window position to be set to negative coordinates; https://reviewboard.mozilla.org/r/114938/#review116740 > You could assert that the coordinates are in some rough range, so that we're more likely to notice if something unusual is happening. Up to you. We should definitely do an assert here. 'print' statements aren't that helpful, and I assume those are left-overs from debugging?
Comment on attachment 8840426 [details] Bug 1340775 - Allow window position to be set to negative coordinates; https://reviewboard.mozilla.org/r/114938/#review116740 > We should definitely do an assert here. 'print' statements aren't that helpful, and I assume those are left-overs from debugging? I believe ato's just using the print statements for logging, which I'm in favour of if it makes the Python test log clearer.
Comment on attachment 8840426 [details] Bug 1340775 - Allow window position to be set to negative coordinates; https://reviewboard.mozilla.org/r/114938/#review116740 > I believe ato's just using the print statements for logging, which I'm in favour of if it makes the Python test log clearer. Yes, the print statements are deliberate – they are very useful for this particular test since the output varies a lot from environment to environment, as well as from platform to platform. I will go ahead and add some kind of assertion as suggested here, but I warn you that it will be very rough and possibly prone to fail in developer environments I haven’t tested.
(In reply to Andreas Tolfsen from comment #3) > jimm: When Firefox is maximised on Windows, window.screenX reports -8 px. > Is this expected behaviour or a bug on our part? It appears that Blink > reports 0 px in the same situation, which leads me to think there is an > interop issue here. > > What are the expected values of window.screenX/window.screenY when a browser > is maximised? The offset native window sounds right, the window origin sits off the desktop. You can see this in Spy by inspecting any maximized window. window.screenX/window.screenY being off as a result sounds like a bug that should be fixed.
Flags: needinfo?(jmathies)
Comment on attachment 8840426 [details] Bug 1340775 - Allow window position to be set to negative coordinates; https://reviewboard.mozilla.org/r/114938/#review116740 > Yes, the print statements are deliberate – they are very useful for this particular test since the output varies a lot from environment to environment, as well as from platform to platform. > > I will go ahead and add some kind of assertion as suggested here, but I warn you that it will be very rough and possibly prone to fail in developer environments I haven’t tested. maja_zf: I have added the necessary assertions. Will you please take a look?
Attachment #8840426 - Flags: review+ → review?(mjzffr)
Comment on attachment 8840426 [details] Bug 1340775 - Allow window position to be set to negative coordinates; https://reviewboard.mozilla.org/r/114938/#review117550 These assertions are a lot more specific than I was expecting but if they work then great. Thanks, ato.
Attachment #8840426 - Flags: review?(mjzffr) → review+
IIUC, this bug is blocking uplift consideration of bug 1338776 (and dupes). Any chance we can get this landed?
Flags: needinfo?(ato)
Thanks for reminding me, RyanVM. I have requested landing through autoland.
Flags: needinfo?(ato)
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78b008fb7d12 Allow window position to be set to negative coordinates; r=maja_zf
https://hg.mozilla.org/mozilla-central/rev/78b008fb7d12
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Could you make an uplift request to aurora and beta? This is prerequisite of bug 1338776 uplift.
Flags: needinfo?(ato)
Depends on: 1347675
No longer depends on: 1347675
It's test-only, so it can land without approval.
status-firefox53: --- → affected
status-firefox54: --- → affected
Flags: needinfo?(ato)
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-aurora/rev/e0563eeab404
status-firefox54: affected → fixed
Flags: in-testsuite+
Whiteboard: [checkin-needed-aurora][checkin-needed-beta] → [checkin-needed-beta]
Product: Testing → Remote Protocol
Moving bug to Testing::Marionette Client and Harness component per bug 1815831.
Component: Marionette → Marionette Client and Harness
Product: Remote Protocol → Testing
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: