Window.open() complete overhaul #12425

dylanosrc · 2022-01-26T07:16:57Z

Summary

This is a complete overhaul of the window.open() page and accomplishes the following:

Cuts down on the verbosity of the page.
Reorganizes the page to fit the API method subpage template.
Removes misleading or outdated information.
Clarifies the code examples

Things that should be considered:

There was an edit to this page in response to Issue with "Window.open()": Can't make "popup" work #11988 that I left out because I think the issue is better answered now with the rework, but to each their own!
This is my first contribution to MDN, so feedback is appreciated. This should be considered a first draft.

Hopefully I did this right!

Motivation

There is an open issue to overhaul this page. See below.

Supporting details

Related issues

Fixes #4583

Metadata

Adds a new document
Rewrites (or significantly expands) a document
Fixes a typo, bug, or other error

files/en-us/web/api/window/open/index.md

github-actions · 2022-01-27T00:46:17Z

Preview URLs

https://pr12425.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/Window/open

Flaws

None! 🎉

External URLs

URL: /en-US/docs/Web/API/Window/open
Title: Window.open()
on GitHub

Make 'true' a truthy value for window.open boolean features whatwg/html#7425 (1 time) (Note! This may be a new URL 👀)

(this comment was updated 2022-03-21 20:32:03.709636)

Co-authored-by: Joe Medley <[email protected]>

jpmedley · 2022-02-01T17:24:31Z

@dylandog95 Thank you for your patience and perseverance in this PR. Updates to old material are always like pulling teeth.

Restored best practices, FAQ and notes sections for later consideration through a new issue. Retained the removal of the tutorials and references sections since links were broken and the tutorials were painfully out of date. (not sure if I restored this correctly, so forgive my naivety if I didn't)

dylanosrc · 2022-02-01T18:46:49Z

@jpmedley You're very welcome. This has been a great learning opportunity for me, so I am thankful for your patience as well!

dylanosrc · 2022-02-05T21:59:03Z

@jpmedley I think I have addressed all requested changes here. Is there anything else you need to me to do to resolve this PR?

Thanks

jpmedley · 2022-02-08T15:04:20Z

@dylandog95 sorry for the delay.

I thought we had decided to restore the removed content. It looks like it's still there.

dylanosrc · 2022-02-11T21:45:52Z

@jpmedley I think there is a misunderstanding or I did something wrong in adding back the latter sections. I merged back the Best Practices, FAQ, Usability Issues, Glossary, and Notes sections while retaining the rewrites that we decided were good to Summary, Syntax, Examples, and Window Features. Looking at the preview, it appears my committed changes did take effect.

wbamberg · 2022-02-18T00:55:54Z

@jpmedley I think there is a misunderstanding or I did something wrong in adding back the latter sections. I merged back the Best Practices, FAQ, Usability Issues, Glossary, and Notes sections while retaining the rewrites that we decided were good to Summary, Syntax, Examples, and Window Features. Looking at the preview, it appears my committed changes did take effect.

I think perhaps one thing that's confusing is that in his review, Joe asked for the window features UI bits, like menubar, to be restored, and you replied that you would, but they aren't? e.g. https://github.com/mdn/content/pull/12425/files#r792776272 .

But also, I think we (the MDN review crew) are not being very consistent in how we want to approach this work, and I'm sorry that this is giving you conflicting signals.

IMO, as I said in https://github.com/mdn/content/pull/12425/files#r793284703, we should remove the UI features, because they're no longer supported anywhere. And I think you are handling this correctly in this PR, listing the window features values that are still useful and saying that other values (like the UI features ones) are now just a signal to use a popup. If the consensus is against this I'm happy to keep this stuff, but I'd like to be explicit about this decision. @teoli2003 , @ddbeck , @Elchi3 , opinions?

I also think we should reorganize other sections of this document. It's a mess structurally: we should IMO take "Best practices", "FAQ", "Usability issues", "Glossary", and "Notes", update them and reorganize them as a single "Description" section. But perhaps we should do this in a follow-up PR, so we can get this one merged and discuss those changes separately. I'd be happy for you to take that follow-up on as well, if you still want to!

I have actual review comments as well as these meta-comments, and I'll get to them soon. Thanks for your work on this PR, @dylandog95 !

dylanosrc · 2022-02-22T19:34:23Z

@wbamberg Thank you for the feedback.

I think perhaps one thing that's confusing is that in his review, Joe asked for the window features UI bits, like menubar, to be > restored, and you replied that you would, but they aren't? e.g. https://github.com/mdn/content/pull/12425/files#r792776272

Per a different comment in reference to UI features Joe stated:

I think deprecation is appropriate.

Due to this response I removed the UI features content.

But perhaps we should do this in a follow-up PR, so we can get this one merged and discuss those changes separately. I'd be > happy for you to take that follow-up on as well, if you still want to!

I think this is the plan once we get this PR merged! I am learning a lot about the process here, so I am eager to tackle more issues!

wbamberg · 2022-02-22T22:09:04Z

@wbamberg Thank you for the feedback.

I think perhaps one thing that's confusing is that in his review, Joe asked for the window features UI bits, like menubar, to be > restored, and you replied that you would, but they aren't? e.g. https://github.com/mdn/content/pull/12425/files#r792776272

Per a different comment in reference to UI features Joe stated:

I think deprecation is appropriate.

Due to this response I removed the UI features content.

Oh! I think perhaps by "deprecation is appropriate" Joe might have meant "keep them, but mark them as deprecated". Which we do quite a lot in our docs.

But anyway, let's see what the consensus is.

But perhaps we should do this in a follow-up PR, so we can get this one merged and discuss those changes separately. I'd be > happy for you to take that follow-up on as well, if you still want to!

I think this is the plan once we get this PR merged! I am learning a lot about the process here, so I am eager to tackle more issues!

Good!

ddbeck · 2022-02-23T18:53:18Z

For the explicit decision question:

IMO, as I said in https://github.com/mdn/content/pull/12425/files#r793284703, we should remove the UI features, because they're no longer supported anywhere. And I think you are handling this correctly in this PR, listing the window features values that are still useful and saying that other values (like the UI features ones) are now just a signal to use a popup. If the consensus is against this I'm happy to keep this stuff, but I'd like to be explicit about this decision. @teoli2003 , @ddbeck , @Elchi3 , opinions?

I'm pretty sure I agree with this approach. If all of the window features had been tracked in browser compat data, their removal from browsers would've prompted removal from BCD and the page. For legacy values that still work (but behave like some other value) for compatibility, then I might suggest reporting that in BCD as an alternative name. But either way, I think it deserves little, if any, mention in the body text today.

wbamberg

Sorry to be slow to get to this @dylandog95 .

I had a few comments. I think we have agreement that it's fine to remove those UI features bits. So if we can resolve the comments here we can merge this and then maybe have another PR to deal with the second half of this page.

Thanks again for your patience!

files/en-us/web/api/window/open/index.md

wbamberg · 2022-02-17T23:47:51Z

files/en-us/web/api/window/open/index.md

-var window = window.open(url, windowName, [windowFeatures]);
+open();
+open(url);
+open(url, windowName);


Is there a reason not to use target , as the spec does, rather than windowName?

There is no reason, other than it being a hold over from the old page. This has been updated.

files/en-us/web/api/window/open/index.md

wbamberg · 2022-03-01T23:32:17Z

files/en-us/web/api/window/open/index.md

-    {{HTMLElement("iframe")}} or tab) into which to load the specified resource; if the
-    name doesn't indicate an existing context, a new window is created and is given the
-    name specified by `windowName`.
+- `windowName` {{optional_inline}}


Again we could call this target. Is there a reason this page wants to use windowName?

See above. Just a hold over.

files/en-us/web/api/window/open/index.md

wbamberg · 2022-03-01T23:43:31Z

files/en-us/web/api/window/open/index.md

-    features include options such as the window's default size and position, whether or
-    not to open a minimal popup window, and so forth. See {{anch("Window features")}}
-    below for documentation of each of the features that can be specified.
+  - : A string containing a comma-separated list of window features in the form _name=value_ — or for boolean features, just _name_. These features include options such as the window's default size and position, whether or not to open a minimal popup window, and so forth. See {{anch("Window features")}} below for descriptions of each feature.


Suggested change

- : A string containing a comma-separated list of window features in the form _name=value_ — or for boolean features, just _name_. These features include options such as the window's default size and position, whether or not to open a minimal popup window, and so forth. See {{anch("Window features")}} below for descriptions of each feature.

- : A string containing a comma-separated list of window features in the form _name=value_ — or for boolean features, just _name_. These features include options such as the window's default size and position, whether or not to open a minimal popup window, and so forth. The following options are supported:

I think it would be better to list the options here, in a nested <dl> (like https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#options), rather than in a separate "Window features" section.

The requested changes have been made in the commit I just sent. This looks cleaner.

wbamberg · 2022-03-01T23:54:16Z

files/en-us/web/api/window/open/index.md

-of the new window as long as it complies with [Same-origin policy](/en-US/docs/Web/Security/Same-origin_policy) security
-requirements.
-
-## Description


I think it is going. to be worth keeping "Description". We will eventually probably want to put some of the content from subsequent sections (Notes, Best practices etc) into there. For now it can give a quick prose overview of the behavior of this API. Something like:

The {{domxref("Window")}} interface's open() method takes a URL as a parameter, and loads the resource it identifies into a new or existing tab or window. The target parameter determines which window or tab to load the resource into, and the windowFeatures parameter can be used to control the size and position of a new window, and to open the new window as a popup with minimal UI features.

Note that remote URLs won't load immediately. When window.open() returns, the window always contains about:blank. The actual fetching of the URL is deferred and starts after the current script block finishes executing. The window creation and the loading of the referenced resource are done asynchronously.

...or something like that anyway.

Okay, I have added the description back onto the page.

wbamberg · 2022-03-01T23:57:40Z

files/en-us/web/api/window/open/index.md


-    > **Note:** For accessibility reasons, it is strongly encouraged to set
-    > this feature always on.
+> **Note:** If a popup is requested and no position features are specified, then the left and top coordinates of the new window will be offset 22 pixels from the original window. If the original window is maximized, the window will also be maximized.


This thing about 22 pixels seems to be Firefox-specific. Perhaps we could just remove the whole note?

wbamberg · 2022-03-01T23:59:44Z

files/en-us/web/api/window/open/index.md

 - [Obsolete features](/en-US/docs/Web/API/Window/open/Obsolete_features)
- [Privileged
-  features](/en-US/docs/Web/API/Window/open/Privileged_features)
+- [Privileged features](/en-US/docs/Web/API/Window/open/Privileged_features)


We should delete both these pages and these links.

These have been deleted through Yarn per instructions.

Co-authored-by: wbamberg <[email protected]>

wbamberg · 2022-03-11T23:51:45Z

Thanks for the update @dylandog95 . There were some other comments from my review: are you still planning to come back to them, or did you have any questions about them?

Co-authored-by: wbamberg <[email protected]>

dylanosrc · 2022-03-15T04:17:52Z

Thanks for the review @wbamberg! I started doing this and fell ill... but I am back good as new! I think I addressed all your requested changes in my latest commit.

wbamberg

Sorry to hear you've been sick @dylandog95 . I hope you didn't feel pressured to complete this PR!

Anyway, I've fixed the merge conflicts and think this looks great now, much better than what we have. I think we should have a follow-up to organize and update all the Notes/FAQ/etc sections, and I'd be happy for you to work on that if you still want to. In the meantime I think we should merge this PR.

@jpmedley , this has a pending review from you, did you want to re-review this PR?

removal of UI windowFeatures was decided to be OK

wbamberg · 2022-03-21T20:35:01Z

I'm merging this so it doesn't rot again!

Thanks for all your work on it @dylandog95 . I think it looks a lot better. If you still want to work on the next set of improvements to this page, please let me know! Maybe we could discuss it in #4583 ?

dylanosrc added 2 commits January 25, 2022 19:31

Fixes #4583

b5b4952

Merge branch 'main' of https://github.com/dylandog95/content

fadfa7b

dylanosrc requested a review from a team as a code owner January 26, 2022 07:16

dylanosrc requested review from jpmedley and removed request for a team January 26, 2022 07:16

github-actions bot added the Content:WebAPI Web API docs label Jan 26, 2022

jpmedley previously requested changes Jan 26, 2022

View reviewed changes

dylanosrc and others added 2 commits January 26, 2022 22:04

Apply suggestions from code review

f144764

Co-authored-by: Joe Medley <[email protected]>

Update files/en-us/web/api/window/open/index.md

e5a1e46

Co-authored-by: Joe Medley <[email protected]>

dylanosrc added 3 commits February 1, 2022 09:47

Update index.md

bb007c8

Removed deprecated UI features

50acd99

Merge branch 'main' into main

d0fb5f0

wbamberg requested changes Mar 2, 2022

View reviewed changes

Apply suggestions from wbamberg review

e1f0631

Co-authored-by: wbamberg <[email protected]>

responses to wbamberg comments

6ffcb9a

dylanosrc requested a review from a team as a code owner March 15, 2022 04:12

dylanosrc requested review from ddbeck and removed request for a team March 15, 2022 04:12

Update files/en-us/web/api/window/open/index.md

96f5cbb

Co-authored-by: wbamberg <[email protected]>

Merge branch 'main' into main

02dde0b

wbamberg approved these changes Mar 15, 2022

View reviewed changes

wbamberg added 2 commits March 21, 2022 13:13

Merge branch 'main' into main

3708c56

Reinstate addition from PR 13648

70547de

wbamberg merged commit 6274f5d into mdn:main Mar 21, 2022

Josh-Cena mentioned this pull request Jun 28, 2024

Restore mention of popup-controlling features in window.open #34494

Merged

	- : A string containing a comma-separated list of window features in the form _name=value_ — or for boolean features, just _name_. These features include options such as the window's default size and position, whether or not to open a minimal popup window, and so forth. See {{anch("Window features")}} below for descriptions of each feature.
	- : A string containing a comma-separated list of window features in the form _name=value_ — or for boolean features, just _name_. These features include options such as the window's default size and position, whether or not to open a minimal popup window, and so forth. The following options are supported:

Window.open() complete overhaul #12425

Window.open() complete overhaul #12425

Uh oh!

Conversation

dylanosrc commented Jan 26, 2022

Summary

Motivation

Supporting details

Related issues

Metadata

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview URLs

Flaws

External URLs

Uh oh!

jpmedley commented Feb 1, 2022

Uh oh!

dylanosrc commented Feb 1, 2022

Uh oh!

dylanosrc commented Feb 5, 2022

Uh oh!

jpmedley commented Feb 8, 2022

Uh oh!

dylanosrc commented Feb 11, 2022

Uh oh!

wbamberg commented Feb 18, 2022

Uh oh!

dylanosrc commented Feb 22, 2022

Uh oh!

wbamberg commented Feb 22, 2022

Uh oh!

ddbeck commented Feb 23, 2022

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbamberg commented Mar 11, 2022

Uh oh!

dylanosrc commented Mar 15, 2022

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 27, 2022 •

edited

Loading