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

feat: add more info in setWindowOpenHandler details #28518

Merged
merged 14 commits into from Apr 13, 2021
Merged

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Apr 5, 2021

Description of Change

This adds disposition, referrer and postBody to the details object passed to the window open handler.

Builds on #28513, which fixes a number of PostBody-related issues.

Closes #28380.

Checklist

Release Notes

Notes: Added disposition, referrer and postBody to the details object passed to the window open handler registered with setWindowOpenHandler.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 5, 2021
@nornagon nornagon added the semver/minor backwards-compatible functionality label Apr 5, 2021
@nornagon
Copy link
Member Author

nornagon commented Apr 5, 2021

This should perhaps be target/12-x-y too

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Apr 6, 2021

Would it be possible to add the parsed features as well? Originally I opened #28377 before I realized setWindowOpenHandler was lacking additional information too.

@nornagon
Copy link
Member Author

nornagon commented Apr 6, 2021

Would it be possible to add the parsed features as well? Originally I opened #28377 before I realized setWindowOpenHandler was lacking additional information too.

parsed features is a little more complicated because:

  1. there's already a property called features in the details object which contains the raw features string, and
  2. parsing the features currently splits the results between options, webPreferences and additionalFeatures, which isn't really a great API and I'd rather not propagate that bad API to the new method.

If it helps, the code for parsing the features isn't much more complicated than features.split(","):

export function parseCommaSeparatedKeyValue (source: string, useSoonToBeDeprecatedBehaviorForBareKeys: boolean) {
const bareKeys = [] as string[];
const parsed = {} as { [key: string]: any };
for (const keyValuePair of source.split(',')) {
const [key, value] = keyValuePair.split('=').map(str => str.trim());
if (useSoonToBeDeprecatedBehaviorForBareKeys && value === undefined) {
if (key) { bareKeys.push(key); }
continue;
}
parsed[key] = coerce(key, value);
}
return { parsed, bareKeys };
}

@Prinzhorn
Copy link
Contributor

@nornagon thanks for looking into this, I'll probably use JSON then if I can use any string. I'm in control of the features string anyway.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 12, 2021
@release-clerk
Copy link

release-clerk bot commented Apr 13, 2021

Release Notes Persisted

Added disposition, referrer and postBody to the details object passed to the window open handler registered with setWindowOpenHandler.

@jkleinsc jkleinsc deleted the window-open-opts branch April 13, 2021 19:35
@trop
Copy link
Contributor

trop bot commented Apr 13, 2021

I was unable to backport this PR to "13-x-y" cleanly;
you will need to perform this backport manually.

VerteDinde pushed a commit that referenced this pull request May 21, 2021
* fix: invoke the window open handler for _blank links

* feat: add disposition to setWindowOpenHandler details

* fix: pass postData to new-window event

* postData can be heterogeneous

* fix type of postBody

* fix type of UploadFile and UploadRawData to be discriminated unions

* exclude the empty string from additionalFeatures

* add a test

* add postBody and referrer to setWindowOpenHandler args

* appease typescript

* Update api-browser-window-spec.ts

* update snapshots
@trop
Copy link
Contributor

trop bot commented May 21, 2021

@VerteDinde has manually backported this PR to "master", please check out #29276

@trop
Copy link
Contributor

trop bot commented May 21, 2021

@VerteDinde has manually backported this PR to "13-x-y", please check out #29276

@trop
Copy link
Contributor

trop bot commented May 21, 2021

@VerteDinde has manually backported this PR to "13-x-y", please check out #29277

zcbenz pushed a commit that referenced this pull request May 24, 2021
* fix: invoke the window open handler for _blank links

* feat: add disposition to setWindowOpenHandler details

* fix: pass postData to new-window event

* postData can be heterogeneous

* fix type of postBody

* fix type of UploadFile and UploadRawData to be discriminated unions

* exclude the empty string from additionalFeatures

* add a test

* add postBody and referrer to setWindowOpenHandler args

* appease typescript

* Update api-browser-window-spec.ts

* update snapshots

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
@trop trop bot added the merged/13-x-y label May 24, 2021
Prinzhorn added a commit to Prinzhorn/electron that referenced this pull request Jul 29, 2021
Since the introduction of `setWindowOpenHandler` `features` is no longer parsed but passed as a raw string (see electron#28518 (comment)). I've switched to JSON to pass data through `window.open` for my multi-window application. The docs should reflect that this can be an arbitrary string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Add {options, disposition, additionalFeatures, referrer, postData} to setWindowOpenHandler
5 participants