Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document conditions for popup-opening by window.open, and BarProp values #10339

Merged
merged 23 commits into from
Nov 9, 2021

Conversation

arai-a
Copy link
Contributor

@arai-a arai-a commented Nov 7, 2021

for whatwg/html#6530

  • Added popup feature
  • Added description about requesting popup
  • Changed examples not to use the backward-compat-only UI parts features
  • Updated BarProp.visible to reflect popup request
  • Moved outerWidth/outerHeight to obsolete features page

Summary

Update windowFeatures parameter behavior for window.open, and also BarProp.visible behavior, to follow the spec change in whatwg/html#6530.

Motivation

Reflect the latest spec.

Supporting details

Firefox: fixed in 96 (currently Nightly) https://bugzilla.mozilla.org/show_bug.cgi?id=1701001
Chrome: in progress https://bugs.chromium.org/p/chromium/issues/detail?id=1192701
Safari: bug is filed https://bugs.webkit.org/show_bug.cgi?id=223751

Related issues

Metadata

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

…Prop values

for whatwg/html#6530

 * Added popup feature
 * Added description about requesting popup
 * Changed examples not to use the backward-compat-only UI parts features
 * Updated BarProp.visible to reflect popup request
 * Moved outerWidth/outerHeight to obsolete features page
@arai-a arai-a requested a review from a team as a code owner November 7, 2021 01:25
@arai-a arai-a requested review from sideshowbarker and removed request for a team November 7, 2021 01:25
@github-actions github-actions bot added the Content:WebAPI Web API docs label Nov 7, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2021

Preview URLs

Flaws

Note! 2 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/Window/open/Obsolete_features
Title: Obsolete features
on GitHub
Flaw count: 2

  • broken_links:
    • Is currently http:// but can become https://
    • Is currently http:// but can become https://

External URLs

URL: /en-US/docs/Web/API/BarProp/visible
Title: BarProp.visible
on GitHub

No new external URLs


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


URL: /en-US/docs/Web/API/Window/open/Obsolete_features
Title: Obsolete features
on GitHub

No new external URLs

(this comment was updated 2021-11-08 08:39:53.537455)

Remove unnecessary hash for fragment
@arai-a
Copy link
Contributor Author

arai-a commented Nov 7, 2021

Sorry, my description was unclear.

for "popup" feature, "not-present" and "present with disabled" has different semantics.
"not-present" fallback to other conditions, and "present with disabled" explicitly "don't request" without checking other features.
https://html.spec.whatwg.org/#popup-window-is-requested

@sideshowbarker
Copy link
Collaborator

Sorry, my description was unclear.

for "popup" feature, "not-present" and "present with disabled" has different semantics.
"not-present" fallback to other conditions, and "present with disabled" explicitly "don't request" without checking other features.
https://html.spec.whatwg.org/#popup-window-is-requested

Ok, thanks ー I'll make another edit to re-correct that

@arai-a
Copy link
Contributor Author

arai-a commented Nov 7, 2021

the basic algorithm that the document is supposed to cover is that:

  • if "popup" is present, then:
    • if enabled (boolean value is true), then request popup
    • otherwise, do not request popup
  • otherwise:
    • if windowFeatures is empty, then do not request popup
    • otherwise:
      • if there's any other feature than noopener and noreferer, then request popup
        • (this has more complex backward-compat condition about UI features, but I don't want to describe it in the document, for simplicity)
      • otherwise, do not request popup

so I mentioned about popup in position and size (left/top/width/height),
and not mentioned about popup in noopener/noreferer.

the "not present" case is covered by "Otherwise" -> "Otherwise, no popup is requested.", that's after "Specifying any features in the _windowFeatures_ parameter other than `noopener` or `noreferer` has the effect of also requesting a popup." part.
so I think it's better omitting the "not present" case from the first section.
@arai-a
Copy link
Contributor Author

arai-a commented Nov 7, 2021

oh, I thought the description becomes comment instead of commit message.

anyway, thank you for reviewing :)
I think it's better omitting the "not present" case from the first section, and let the later "Otherwise" handle it,
so that it checks other features.

@arai-a
Copy link
Contributor Author

arai-a commented Nov 7, 2021

the difference is that:
width=500,height=500 (not present) requests popup, and
width=500,height=500,popup=no (present and disabled) doesn't request popup.

(in both case, whether to handle width/height, and whether to open new window or tab, are browser-dependent)

@sideshowbarker
Copy link
Collaborator

oh, I thought the description becomes comment instead of commit message.

anyway, thank you for reviewing :) I think it's better omitting the "not present" case from the first section, and let the later "Otherwise" handle it, so that it checks other features.

Please go ahead and commit a change for that

@arai-a
Copy link
Contributor Author

arai-a commented Nov 7, 2021

done in dbd3667

@sideshowbarker
Copy link
Collaborator

@arai-a Please re-review the current state of the content in the branch. If it all looks accurate to you, then I will merge it later this afternoon. (Right now I’m going to take a break for a couple hours or so.)

@arai-a
Copy link
Contributor Author

arai-a commented Nov 7, 2021

Thank you again!

tweaked the position/size description not to mention boolean feature handling.
otherwise it looks good :)

@sideshowbarker
Copy link
Collaborator

Sorry! Only just now realized I’d forgotten to merge this…

@sideshowbarker sideshowbarker changed the title Standardize the condition for opening a popup by window.open, and BarProp values Document conditions for popup-opening by window.open, and BarProp values Nov 9, 2021
@sideshowbarker sideshowbarker merged commit df7ff9b into mdn:main Nov 9, 2021
@sideshowbarker
Copy link
Collaborator

@arai-a Really great work (including the related spec work you did) — congrats on landing your first docs change here, and welcome aboard 🎉

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants