Closed Bug 1658928 Opened 6 months ago Closed 5 months ago

Make "WebDriver:GetCurrentURL" Fission compatible

Categories

(Testing :: Marionette, task, P1)

Default
task

Tracking

(firefox81 fixed)

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

(Whiteboard: [marionette-fission-mvp], [wptsync upstream])

Attachments

(2 files, 1 obsolete file)

This command should work through the JSWindowActor pair to simplify the behavior for both content and chrome.

Note that since bug 1368492 landed we do not have an end-point in listener.js, which is not pretty helpful. Also because for reftests running on Android this.driver.curBrowser.currentURI returns null. Having the command been forwarded to the content process should give us the expected data.

As such I will attach two patches. The first will partly revert the former changes, and the second one will add the Actor support.

Having the patches from bug 1654628 landed first, will kinda help here. Marking as blocking.

Depends on: 1654628

By moving out the "currentURI" getter from the Browser class,
and making it available on the driver other parts of Marionette
like reftests can make use of the "getCurrentURI" helper method.
Hereby in case of content context the URL needs to be retrieved
from the framescript.

Also a couple of methods expressed the passed-around data as
instance of nsIURI where by it was a string or an instance of
URL. The patch fixes that to always pass around nsIURI instances,
and using it's appropriate properties when handling cookies.

The patch adds the handling of the current URI to the JSWindowActor.
Given that the documentURI is already available via the currentWindowGlobal
of the parent actor, it can be returned immediately without having
to do any IPC communication.

Depends on D87572

Actually using nsIURI isn't that elegant as using URL(). Reason is that the former raises exceptions when accessing properties like host that would not match. As example a NS_ERROR_FAILURE is raised when trying to get the host from nsIURI("about:blank").

When using new URL() the host is just an empty string and way better to handle. Given that I will revert parts of the first patch to make use of URL.

Attachment #9170900 - Attachment is obsolete: true
Attachment #9170899 - Attachment description: Bug 1658928 - [marionette] Refactor "WebDriver:GetCurrentURL". → Bug 1658928 - [marionette] Make "WebDriver:GetCurrentURL" Fission compatible.
Summary: Port WebDriver:GetCurrentUrl to JSWindowActor → Make "WebDriver:GetCurrentURL" Fission compatible
Attachment #9170899 - Attachment description: Bug 1658928 - [marionette] Make "WebDriver:GetCurrentURL" Fission compatible. → Bug 1658928 - [marionette] Refactor "WebDriver:GetCurrentURL".
Attachment #9170899 - Attachment description: Bug 1658928 - [marionette] Refactor "WebDriver:GetCurrentURL". → Bug 1658928 - [marionette] Make "WebDriver:GetCurrentURL" Fission compatible.
Whiteboard: [marionette-fission-mvp]
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/391624a4819f
[marionette] Make "WebDriver:GetCurrentURL" Fission compatible. r=marionette-reviewers,maja_zf
https://hg.mozilla.org/integration/autoland/rev/d72e8a597ae4
[wdspec] Improve tests for getCurrentURL and add site-isolation test. r=maja_zf
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/25192 for changes under testing/web-platform/tests
Whiteboard: [marionette-fission-mvp] → [marionette-fission-mvp], [wptsync upstream]
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.