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

Use origin of source document for MessageEvent resulting from postMessage #22888

Closed
jdm opened this issue Feb 13, 2019 · 12 comments
Closed

Use origin of source document for MessageEvent resulting from postMessage #22888

jdm opened this issue Feb 13, 2019 · 12 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 13, 2019

Right now Window::post_message passes None as the value that will initialize MessageEvent.origin. The spec says this should be the origin of the source document that posted the message instead. We will want to add a new origin argument to Window::post_message, and store the origin in ScriptMsg::PostMessage as well so the code path in DissimilarOriginWindow::post_message also works.

This should allow tests/wpt/web-platform-tests/dom/events/EventListener-incumbent-global-1.sub.html to make more progress.

Code: components/script/dom/window.rs, components/script/dom/dissimilaroriginwindow.rs, components/script/script_thread.rs, components/script_traits/lib.rs
Test: ./mach test-wpt tests/wpt/web-platform-tests/dom/events/EventListener-incumbent-global-1.sub.html

@EllaLiu5
Copy link

@EllaLiu5 EllaLiu5 commented Feb 25, 2019

Assign this to me please.

@KiChjang KiChjang added the C-assigned label Feb 25, 2019
@jdm
Copy link
Member Author

@jdm jdm commented Mar 20, 2019

@EllaLiu5 Have you made any progress? Do you have any questions?

@jdm jdm removed the C-assigned label May 22, 2019
@yashpatel5400
Copy link

@yashpatel5400 yashpatel5400 commented Jun 12, 2019

I'd be interested in trying this out!

@CYBAI CYBAI added the C-assigned label Jun 12, 2019
@jdm
Copy link
Member Author

@jdm jdm commented Jul 12, 2019

@yashpatel5400 Did you make any progress? Are you still working on this?

@mmiecz
Copy link
Contributor

@mmiecz mmiecz commented Sep 18, 2019

Hey, it appears that no one is working on this one. If that's ok I can look into this task.

@jdm
Copy link
Member Author

@jdm jdm commented Sep 18, 2019

Please do!

@jdm
Copy link
Member Author

@jdm jdm commented Oct 10, 2019

@mmiecz Have you made any progress on this?

@mmiecz
Copy link
Contributor

@mmiecz mmiecz commented Oct 10, 2019

Yes, the test is passing now, but I still have to clean up the code ( remove unwraps, etc. ). I will create PR soon, so you can make some comments, because I am not entirely sure about the solution.

@mmiecz
Copy link
Contributor

@mmiecz mmiecz commented Oct 15, 2019

Hi, I am having some hardware problems since I’ve upgraded recently and I can’t boot to Linux where I have my servo code :( I am trying to resolve this issue right no, sorry for the delay!

@mmiecz
Copy link
Contributor

@mmiecz mmiecz commented Oct 23, 2019

I've created a PR but I see it's already done?

@gterzian
Copy link
Member

@gterzian gterzian commented Oct 24, 2019

@mmiecz Yes you're right, this was already done in #23637, sorry I wasn't aware an issue was open for this.

I think you could re-use some of what you've learned so far on this related issue: #24454

@jdm
Copy link
Member Author

@jdm jdm commented Jan 21, 2020

Closing since this was fixed by #23637.

@jdm jdm closed this Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants
You can’t perform that action at this time.