Should 'true' count as truthy in parsing windowFeatures booleans? #7399

mfreed7 · 2021-12-08T20:27:49Z

If value is the empty string, then return true.
If value is "yes", then return true.
Let parsed be the result of parsing value as an integer.
If parsed is an error, then set it to 0.
Return false if parsed is 0, and true otherwise.

...which means that something like window.open('about:blank','','popup=true') interprets the popup=true to mean popup is false. Not very intuitive.

Should we add "true" as a value that results in true?

This was brought up as a bug against Chromium related to the implementation of the new popup standardization work.

The text was updated successfully, but these errors were encountered:

annevk · 2021-12-09T07:50:55Z

This hasn't been a problem for the other features thus far, e.g., noopener. @zcorpan probably knows the answer here.

zcorpan · 2021-12-09T14:06:45Z

See #2600 -- the spec matched what browsers did for the legacy booleans.

I don't mind supporting true.

Do people expect popup=popup to also mean true?

zcorpan · 2021-12-09T14:07:47Z

cc @cdumez

cdumez · 2021-12-09T15:44:09Z

WebKit already allows "1" / "on" / "yes". I don't mind adding support for "true" too if other browser engines do the same.
"true" seems like a logical value for a boolean.

I don't really like the idea of popup=popup meaning true though.

zcorpan · 2021-12-17T11:45:29Z

"on" wasn't specified in #4079 . Testing with http://software.hixie.ch/utilities/js/live-dom-viewer/saved/9908 webkit/chromium/gecko do not support it.

log: noopener=0: [object Window]
log: noopener=1: null
log: noopener=on: [object Window]
log: noopener=yes: null
log: noopener=true: [object Window]
log: noopener=noopener: [object Window]

Fixes #7399.

cdumez · 2021-12-17T15:24:32Z

"on" wasn't specified in #4079 . Testing with http://software.hixie.ch/utilities/js/live-dom-viewer/saved/9908 webkit/chromium/gecko do not support it.
log: noopener=0: [object Window]
log: noopener=1: null
log: noopener=on: [object Window]
log: noopener=yes: null
log: noopener=true: [object Window]
log: noopener=noopener: [object Window]

Indeed, it looks like I was wrong, we allow "on" only for some features: center, resizable, status, scroll. For noopener, it seems we allow "yes" and any numeric value other than 0 (not sure why noopener is not treated like our other boolean features).

annevk · 2021-12-20T08:08:18Z

The one concern I have here is if we do want to switch to using a dictionary for "cool window features" (such as noopener, noreferrer, and popup) this seems a lot less worth it.

domenic · 2022-01-11T19:28:20Z

Here is my proposal for "switching to a dictionary for cool window features": #7485

I don't really want to block small improvements everyone is on-board with in favor of larger changes though, so take it with a grain of salt...

zcorpan · 2022-02-18T10:55:11Z

@mfreed7 @cdumez do you still think this is a worthwhile change, considering there's now a proposed modern API (#7485)?

cdumez · 2022-02-18T17:14:18Z

This is a very small amount of work to treat 'true' as truthy so I don't mind doing it, even if there is modern API proposal.

mfreed7 · 2022-02-21T19:07:07Z

This is a very small amount of work to treat 'true' as truthy so I don't mind doing it, even if there is modern API proposal.

+1 - I support this. The modern syntax is nice, but I think it can coexist with this small change to the old syntax.

domenic · 2022-02-27T21:02:01Z

Great, so what's required now before landing #7425 is web platform tests.

Per the triage meeting, as a bonus, it'd be great if we also made sure the web platform tests were comprehensive for "on" / "off", "yes" / "no", and non-zero numeric values as well, since it sounds like there's probably some divergence from the spec there, and perhaps interop issues.

mfreed7 · 2022-02-28T21:36:16Z

Great, so what's required now before landing #7425 is web platform tests.

Per the triage meeting, as a bonus, it'd be great if we also made sure the web platform tests were comprehensive for "on" / "off", "yes" / "no", and non-zero numeric values as well, since it sounds like there's probably some divergence from the spec there, and perhaps interop issues.

I'm happy to tackle the WPT enhancements as part of implementing this change in Chromium, assuming we can land #7425 to make that easier for me.

zcorpan · 2022-02-28T23:44:54Z

@mfreed7 great! This file (used by noopener and noreferrer tests) may be of interest in addition to the test for popup

html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/resources/tokenization-noopener-noreferrer.js

We can land the spec change when there's a CL in review with tests.

mfreed7 · 2022-03-01T00:45:09Z

@mfreed7 great! This file (used by noopener and noreferrer tests) may be of interest in addition to the test for popup

html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/resources/tokenization-noopener-noreferrer.js

We can land the spec change when there's a CL in review with tests.

Thanks for the pointer. I thought about adding another set of cases there for =true but ended up just writing a fresh test that looks at just values for noopener, noreferrer, and popup. (The other boolean window features seem like overkill to test, and a bit nebulous given our recent changes to how popups work.)

One question did come out of my new test though. What do we think about these?

  noreferrer=true    // With the new PR, this will result in noreferrer being *true*
  noreferrer="true"  // The quotes cause a parse error, and noreferrer is *false*
  noreferrer='true'  // Same here, *false*

Currently, the rules, and browsers, treat quotation marks as a parse error and the quoted values are interpreted as false. But akin to the rationale for making property=true enable property, I would think we'd similarly want to make property="true" work as well? Perhaps there's more context for why we don't allow quotes here, whereas we're pretty lenient on quotes most other places in the web platform?

zcorpan · 2022-03-01T08:33:53Z

I think they should continue to be false. If you start allowing quotation marks you make the features tokenizer more complicated, at least if you want it to make sense, e.g.:

noopener=", popup=1, "

Does this have popup=1 as a feature, or no?

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. [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

mfreed7 · 2022-03-02T22:43:12Z

I think they should continue to be false. If you start allowing quotation marks you make the features tokenizer more complicated, at least if you want it to make sense, e.g.:
noopener=", popup=1, "
Does this have popup=1 as a feature, or no?

Right - that's a great point. Ok, I agree with you, quotes should parse as false.

Ok, quick and dirty tests have been added here: web-platform-tests/wpt#33029
Comments appreciated. I will wait to land them until I've sent an intent to ship for Chromium, and the spec has landed.

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. [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

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

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 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3495659 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#977722}

Fixes #7399.

…dowFeatures, a=testonly Automatic update from web-platform-tests Add "true" to truthy values list for windowFeatures 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 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3495659 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#977722} -- wpt-commits: 2026041556d8c61e167d0a0d3d0bbbeaf99dcd70 wpt-pr: 33029

Fixes whatwg#7399.

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 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3495659 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#977722} NOKEYCHECK=True GitOrigin-RevId: 0c49a8d7e96bbad1102c934687836d3795073603

mfreed7 added the agenda+ To be discussed at a triage meeting label Dec 8, 2021

zcorpan added a commit that referenced this issue Dec 17, 2021

Make 'true' a truthy value for window.open boolean features

1f74334

Fixes #7399.

zcorpan mentioned this issue Dec 17, 2021

Make 'true' a truthy value for window.open boolean features #7425

Merged

3 tasks

past mentioned this issue Jan 14, 2022

Upcoming HTML standard issue triage meeting on 1/13/2022 #7385

Closed

past removed the agenda+ To be discussed at a triage meeting label Feb 3, 2022

past mentioned this issue Feb 3, 2022

Upcoming HTML standard issue triage meeting on 2/3/2022 #7488

Closed

chromium-wpt-export-bot mentioned this issue Mar 2, 2022

Add "true" to truthy values list for windowFeatures web-platform-tests/wpt#33029

Merged

past mentioned this issue Mar 3, 2022

Upcoming HTML standard issue triage meeting on 3/3/2022 #7581

Closed

domenic closed this as completed in #7425 Mar 7, 2022

domenic pushed a commit that referenced this issue Mar 7, 2022

Make 'true' a truthy value for window.open boolean features

ca752b0

Fixes #7399.

mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022

Make 'true' a truthy value for window.open boolean features

3cfea94

Fixes whatwg#7399.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Should 'true' count as truthy in parsing windowFeatures booleans? #7399

Should 'true' count as truthy in parsing windowFeatures booleans? #7399

mfreed7 commented Dec 8, 2021

annevk commented Dec 9, 2021

zcorpan commented Dec 9, 2021

zcorpan commented Dec 9, 2021

cdumez commented Dec 9, 2021

zcorpan commented Dec 17, 2021 •

edited

Loading

cdumez commented Dec 17, 2021

annevk commented Dec 20, 2021

domenic commented Jan 11, 2022

zcorpan commented Feb 18, 2022

cdumez commented Feb 18, 2022

mfreed7 commented Feb 21, 2022

domenic commented Feb 27, 2022

mfreed7 commented Feb 28, 2022

zcorpan commented Feb 28, 2022

mfreed7 commented Mar 1, 2022

zcorpan commented Mar 1, 2022

mfreed7 commented Mar 2, 2022

Should 'true' count as truthy in parsing windowFeatures booleans? #7399

Should 'true' count as truthy in parsing windowFeatures booleans? #7399

Comments

mfreed7 commented Dec 8, 2021

annevk commented Dec 9, 2021

zcorpan commented Dec 9, 2021

zcorpan commented Dec 9, 2021

cdumez commented Dec 9, 2021

zcorpan commented Dec 17, 2021 • edited Loading

cdumez commented Dec 17, 2021

annevk commented Dec 20, 2021

domenic commented Jan 11, 2022

zcorpan commented Feb 18, 2022

cdumez commented Feb 18, 2022

mfreed7 commented Feb 21, 2022

domenic commented Feb 27, 2022

mfreed7 commented Feb 28, 2022

zcorpan commented Feb 28, 2022

mfreed7 commented Mar 1, 2022

zcorpan commented Mar 1, 2022

mfreed7 commented Mar 2, 2022

zcorpan commented Dec 17, 2021 •

edited

Loading