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 event.senderFrame property returning the webFrameMain #26764

Merged
merged 1 commit into from Dec 9, 2020

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Dec 1, 2020

Description of Change

Also add missing event.processId, which needs to be passed to webFrameMain.fromId().

Checklist

Release Notes

Notes: Added event.senderFrame property returning the originating webFrameMain of the IPC message.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Dec 1, 2020
@miniak miniak marked this pull request as ready for review December 1, 2020 18:47
@nornagon nornagon added api-review/requested 🗳 semver/minor backwards-compatible functionality labels Dec 1, 2020
@miniak miniak changed the title feat: add event.processId to be usable with webFrameMain.fromId() feat: add event.frame property returning the originating webFrameMain Dec 1, 2020
@miniak miniak force-pushed the miniak/event-process-id branch 3 times, most recently from 80275f0 to 5da8a4d Compare December 1, 2020 19:40
@miniak miniak changed the title feat: add event.frame property returning the originating webFrameMain feat: add event.senderFrame property returning the webFrameMain Dec 1, 2020
@miniak miniak self-assigned this Dec 1, 2020
docs/api/structures/ipc-main-event.md Outdated Show resolved Hide resolved
docs/api/web-frame-main.md Show resolved Hide resolved
lib/browser/api/web-contents.ts Outdated Show resolved Hide resolved
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

Copy link
Member

@samuelmaddock samuelmaddock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getter is going to be very handy. Nice!

Object.defineProperty(event, 'senderFrame', {
get: () => webFrameMain.fromId(processId, frameId)
});
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!

@miniak miniak force-pushed the miniak/event-process-id branch 2 times, most recently from 83f8c74 to 0cac7e2 Compare December 2, 2020 02:12
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM as well

@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours and removed new-pr 🌱 PR opened in the last 24 hours labels Dec 2, 2020
@miniak
Copy link
Contributor Author

miniak commented Dec 9, 2020

@nornagon I've noticed that, I had it originally

    dict.Set("processId", frame->GetProcess()->GetID());
    dict.Set("frameId", frame->GetRoutingID());

and then changed it to match your PR to avoid conflict

    dict.Set("frameId", frame->GetRoutingID());
    dict.Set("processId", frame->GetProcess()->GetID());

which broke the snapshot for tests

@nornagon
Copy link
Member

nornagon commented Dec 9, 2020

@miniak the linked PR updates the tests to replace the process id with a placeholder in the snapshot.

@codebytere codebytere merged commit cec6378 into master Dec 9, 2020
@release-clerk
Copy link

release-clerk bot commented Dec 9, 2020

Release Notes Persisted

Added event.senderFrame property returning the originating webFrameMain of the IPC message.

@codebytere codebytere deleted the miniak/event-process-id branch December 9, 2020 23:34
@trop
Copy link
Contributor

trop bot commented Dec 9, 2020

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

@miniak
Copy link
Contributor Author

miniak commented Dec 16, 2020

/trop run backport-to 12-x-y

@trop
Copy link
Contributor

trop bot commented Dec 16, 2020

The backport process for this PR has been manually initiated -
sending your commits to "12-x-y"!

@trop
Copy link
Contributor

trop bot commented Dec 16, 2020

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

@trop
Copy link
Contributor

trop bot commented Dec 16, 2020

@miniak has manually backported this PR to "12-x-y", please check out #27047

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants