Skip to content

Commit

Permalink
Bug 1661495 - [marionette] Don't always set the current content brows…
Browse files Browse the repository at this point in the history
…ing context when registering a new browser. r=marionette-reviewers,maja_zf

Since the patch on bug 1652932 landed in Firefox 80 we always
update the current content browsing context and that now only
when switching to a new window. That leads to an unexpected
change of the current window handle, and as such breaks tests.

Differential Revision: https://phabricator.services.mozilla.com/D88771
  • Loading branch information
whimboo committed Aug 31, 2020
1 parent 7a6bc81 commit 4a28223
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 15 deletions.
14 changes: 8 additions & 6 deletions testing/marionette/driver.js
Expand Up @@ -397,6 +397,8 @@ GeckoDriver.prototype.getBrowsingContext = function(options = {}) {
browsingContext = browsingContext.top;
}

logger.trace(`Using browsing context ${browsingContext?.id}`);

return browsingContext;
};

Expand Down Expand Up @@ -497,8 +499,6 @@ GeckoDriver.prototype.addBrowser = function(win) {
*/
GeckoDriver.prototype.startBrowser = function(window, isNewSession = false) {
this.mainFrame = window;
this.chromeBrowsingContext = this.mainFrame.browsingContext;
this.contentBrowsingContext = null;

this.addBrowser(window);
this.whenBrowserStarted(window, isNewSession);
Expand Down Expand Up @@ -592,8 +592,7 @@ GeckoDriver.prototype.registerBrowser = function(id, be) {
this.curBrowser.register(id, be);
}

this.contentBrowsingContext = BrowsingContext.get(id);
this.wins.set(id, this.contentBrowsingContext.currentWindowGlobal);
this.wins.set(id, BrowsingContext.get(id).currentWindowGlobal);

return id;
};
Expand Down Expand Up @@ -867,10 +866,12 @@ GeckoDriver.prototype.newSession = async function(cmd) {
}

if (this.mainFrame) {
this.chromeBrowsingContext = this.mainFrame.browsingContext;
this.mainFrame.focus();
}

if (this.curBrowser.tab) {
this.contentBrowsingContext = this.curBrowser.contentBrowser.browsingContext;
this.curBrowser.contentBrowser.focus();
}

Expand Down Expand Up @@ -1707,6 +1708,8 @@ GeckoDriver.prototype.setWindowHandle = async function(

this.startBrowser(winProperties.win, false /* isNewSession */);

this.chromeBrowsingContext = this.mainFrame.browsingContext;

if (registerBrowsers && browserListening) {
await registerBrowsers;
const id = await browserListening;
Expand All @@ -1717,8 +1720,6 @@ GeckoDriver.prototype.setWindowHandle = async function(
this.curBrowser = this.browsers[winProperties.id];
this.mainFrame = this.curBrowser.window;

this.chromeBrowsingContext = this.mainFrame.browsingContext;

// Activate the tab if it's a content window.
let tab = null;
if (winProperties.hasTabBrowser) {
Expand All @@ -1729,6 +1730,7 @@ GeckoDriver.prototype.setWindowHandle = async function(
);
}

this.chromeBrowsingContext = this.mainFrame.browsingContext;
this.contentBrowsingContext = tab?.linkedBrowser.browsingContext;
}

Expand Down
@@ -1,2 +1,3 @@
[new_window.py]
disabled: os == "android": Fennec doesn't support opening new windows
disabled:
if os == "android": GeckoView doesn't support opening new windows
Expand Up @@ -4,6 +4,7 @@

from tests.support.asserts import assert_error, assert_success
from tests.support.inline import inline
from tests.support.sync import Poll


def execute_script(session, script, args=None):
Expand All @@ -28,6 +29,24 @@ def test_no_browsing_context(session, closed_window):
assert_error(response, "no such window")


def test_opening_new_window_keeps_current_window_handle(session):
original_handle = session.window_handle
original_handles = session.handles

url = inline("""<a href="javascript:window.open();">open window</a>""")
session.url = url
session.find.css("a", all=False).click()
wait = Poll(
session,
timeout=5,
message="No new window has been opened")
new_handles = wait.until(lambda s: set(s.handles) - set(original_handles))

assert len(new_handles) == 1
assert session.window_handle == original_handle
assert session.url == url


def test_ending_comment(session):
response = execute_script(session, "return 1; // foo")
assert_success(response, 1)
Expand Down
24 changes: 20 additions & 4 deletions testing/web-platform/tests/webdriver/tests/new_window/new_tab.py
@@ -1,4 +1,5 @@
from tests.support.asserts import assert_success
from tests.support.inline import inline

from . import opener, window_name

Expand All @@ -9,7 +10,7 @@ def new_window(session, type_hint=None):
{"type": type_hint})


def test_new_tab(session):
def test_payload(session):
original_handles = session.handles

response = new_window(session, type_hint="tab")
Expand All @@ -21,16 +22,31 @@ def test_new_tab(session):
assert value["type"] == "tab"


def test_new_tab_opens_about_blank(session):
def test_keeps_current_window_handle(session):
original_handle = session.window_handle

response = new_window(session, type_hint="tab")
value = assert_success(response)
assert value["type"] == "tab"

assert session.window_handle == original_handle


def test_opens_about_blank_in_new_tab(session):
url = inline("<p>foo")
session.url = url

response = new_window(session, type_hint="tab")
value = assert_success(response)
assert value["type"] == "tab"

assert session.url == url

session.window_handle = value["handle"]
assert session.url == "about:blank"


def test_new_tab_sets_no_window_name(session):
def test_sets_no_window_name(session):
response = new_window(session, type_hint="tab")
value = assert_success(response)
assert value["type"] == "tab"
Expand All @@ -39,7 +55,7 @@ def test_new_tab_sets_no_window_name(session):
assert window_name(session) == ""


def test_new_tab_sets_no_opener(session):
def test_sets_no_opener(session):
response = new_window(session, type_hint="tab")
value = assert_success(response)
assert value["type"] == "tab"
Expand Down
@@ -1,4 +1,5 @@
from tests.support.asserts import assert_success
from tests.support.inline import inline

from . import opener, window_name

Expand All @@ -9,7 +10,7 @@ def new_window(session, type_hint=None):
{"type": type_hint})


def test_type_with_window(session):
def test_payload(session):
original_handles = session.handles

response = new_window(session, type_hint="window")
Expand All @@ -21,16 +22,31 @@ def test_type_with_window(session):
assert value["type"] == "window"


def test_new_window_opens_about_blank(session):
def test_keeps_current_window_handle(session):
original_handle = session.window_handle

response = new_window(session, type_hint="window")
value = assert_success(response)
assert value["type"] == "window"

assert session.window_handle == original_handle


def test_opens_about_blank_in_new_window(session):
url = inline("<p>foo")
session.url = url

response = new_window(session, type_hint="window")
value = assert_success(response)
assert value["type"] == "window"

assert session.url == url

session.window_handle = value["handle"]
assert session.url == "about:blank"


def test_new_window_sets_no_window_name(session):
def test_sets_no_window_name(session):
response = new_window(session, type_hint="window")
value = assert_success(response)
assert value["type"] == "window"
Expand All @@ -39,7 +55,7 @@ def test_new_window_sets_no_window_name(session):
assert window_name(session) == ""


def test_new_window_sets_no_opener(session):
def test_sets_no_opener(session):
response = new_window(session, type_hint="window")
value = assert_success(response)
assert value["type"] == "window"
Expand Down

0 comments on commit 4a28223

Please sign in to comment.