Skip to content

Commit

Permalink
Add "true" to truthy values list for windowFeatures
Browse files Browse the repository at this point in the history
Prior to this CL, window.open(url, '', 'noopener=true') would treat
'noopener=true' as if noopener is false. This is currently per-spec [1]
but there is a desire [2][3] to change that.

The I2S is here:
https://groups.google.com/a/chromium.org/g/blink-dev/c/ePJ4GE6VzVc/m/urs3-4rHDAAJ

[1] https://html.spec.whatwg.org/multipage/window-object.html#concept-window-open-features-parse-boolean
[2] whatwg/html#7425
[3] whatwg/html#7399

Fixed: 1277613
Change-Id: I5b3a7e985a9bb392c2150846b50369cfcd9b05fa
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Mar 3, 2022
1 parent 206491a commit ef7eb2f
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,48 +26,59 @@
// The explicit popup feature.
["popup", false],
["popup=1", false],
["popup=true", false],
["popup=0", true],

// Other feature alone results in popup.
["location", false],
["location=yes", false],
["location=true", false],
["location=no", false],

["toolbar", false],
["toolbar=yes", false],
["toolbar=true", false],
["toolbar=no", false],

["menubar", false],
["menubar=yes", false],
["menubar=true", false],
["menubar=no", false],

["resizable", false],
["resizable=yes", false],
["resizable=true", false],
["resizable=no", false],
],
"single-2": [
["scrollbars", false],
["scrollbars=yes", false],
["scrollbars=true", false],
["scrollbars=no", false],

["status", false],
["status=yes", false],
["status=true", false],
["status=no", false],

["titlebar", false],
["titlebar=yes", false],
["titlebar=true", false],
["titlebar=no", false],

["close", false],
["close=yes", false],
["close=true", false],
["close=no", false],

["minimizable", false],
["minimizable=yes", false],
["minimizable=true", false],
["minimizable=no", false],

["personalbar", false],
["personalbar=yes", false],
["personalbar=true", false],
["personalbar=no", false],
],
"position": [
Expand Down Expand Up @@ -113,6 +124,8 @@

// The explicit popup feature has priority than others.
["popup=1,location,toolbar,menubar,resizable,scrollbars,status", false],
["popup=yes,location,toolbar,menubar,resizable,scrollbars,status", false],
["popup=true,location,toolbar,menubar,resizable,scrollbars,status", false],
["popup=0,location,toolbar,menubar,resizable,scrollbars", true],
],
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<script>
const channelName = location.search.substr(1),
channel = new BroadcastChannel(channelName);

const haveOpener = window.opener !== null;
const haveReferrer = document.referrer !== null && document.referrer !== "";
const allBarProps = [
window.locationbar.visible,
window.menubar.visible,
window.personalbar.visible,
window.scrollbars.visible,
window.statusbar.visible,
window.toolbar.visible
];
const isPopup = allBarProps.every(x=>!x);

channel.postMessage({haveOpener, haveReferrer, isPopup});

// Because messages are not delivered synchronously and because closing a
// browsing context prompts the eventual clearing of all task sources, this
// document should not be closed until the opener document has confirmed
// receipt.
channel.onmessage = () => window.close();
</script>
3 changes: 3 additions & 0 deletions html/browsers/the-window-object/window-open-noopener.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
{ testDescription: "noopener=1 means the same as noopener",
secondWindowFeatureString: "noopener=1",
shouldReturnWindow: false },
{ testDescription: "noopener=true means the same as noopener",
secondWindowFeatureString: "noopener=true",
shouldReturnWindow: false },
{ testDescription: "noopener=0 means lack of noopener",
secondWindowFeatureString: "noopener=0",
shouldReturnWindow: true },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<!doctype html>
<meta charset=utf-8>
<meta name="timeout" content="long">
<title>window.open() windowFeature value parsing</title>
<link rel="author" href="mailto:masonf@chromium.org">
<link rel="help" href="https://html.spec.whatwg.org/multipage/window-object.html#concept-window-open-features-parse-boolean">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
function testValueGeneric(val, expectTrue, property, testFn) {
const windowFeatureStr = val === "" ? property : `${property}=${val}`;
async_test(t => {
const windowName = '' + Math.round(Math.random()*1e12);
const channel = new BroadcastChannel(windowName);
channel.onmessage = t.step_func_done(e => {
// Send message first so if asserts throw the popup is still closed
channel.postMessage(null);
testFn(e.data);
});
window.open("support/windowFeature-values-target.html?" + windowName, windowName, windowFeatureStr);
},`Test ${windowFeatureStr}, expected interpretation is ${expectTrue ? 'true' : 'false'}`);
}

function testValueForNoReferrer(val, expectTrue) {
testValueGeneric(val, expectTrue, "noreferrer", (data) => {
if (expectTrue) {
assert_false(data.haveReferrer);
assert_false(data.haveOpener);
} else {
assert_true(data.haveReferrer);
assert_true(data.haveOpener);
}
});
}

function testValueForNoOpener(val, expectTrue) {
testValueGeneric(val, expectTrue, "noopener", (data) => {
assert_equals(data.haveOpener, !expectTrue);
});
}

function testValueForPopup(val, expectTrue) {
testValueGeneric(val, expectTrue, "popup", (data) => {
assert_equals(data.isPopup, expectTrue);
});
}

function testValue(val, expectTrue) {
const quotes = val === "" ? [''] : ['','"',"'"];
let noQuotes = true;
for (const quote of quotes) {
const thisExpectTrue = expectTrue && noQuotes;
const thisVal = quote + val + quote;
testValueForNoReferrer(thisVal, thisExpectTrue);
testValueForNoOpener(thisVal, thisExpectTrue);
testValueForPopup(thisVal, thisExpectTrue);
noQuotes = false;
}
}

testValue('',true); // Just the parameter means true
testValue('yes',true); // Yes means true
testValue('true',true); // True means true
testValue('foo',false); // If parsing as an integer is an error, false
testValue('0',false); // 0 is false
testValue('00',false); // 0 is false
testValue('1',true); // Non-zero is true
testValue('99999',true); // Non-zero is true
testValue('-1',true); // Non-zero is true
testValue('1foo',true); // This parses to 1
testValue('0foo',false); // This parses to 0
</script>

0 comments on commit ef7eb2f

Please sign in to comment.