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

Fix : DOMStringMap is no more exposed to Worker #302

Closed
wants to merge 3 commits into from

Conversation

PoojaSanklecha
Copy link
Contributor

No description provided.

@foolip
Copy link
Member

foolip commented Oct 30, 2015

For better or worse, the spec usually relies on the default exposure set, so you can remove Exposed=Window entirely.

If you can also link to https://www.w3.org/Bugs/Public/show_bug.cgi?id=28104 in the commit message and add your name to to the acknowledgements at the end of the spec, I'd be happy to merge this.

@PoojaSanklecha
Copy link
Contributor Author

Actually I have reviewed the source HTML, it has Exposed=Window at arount 7-8 places. So i asked @annevk. He said its better to remover Worker. Should I change again?

@foolip
Copy link
Member

foolip commented Oct 30, 2015

The only other place where it's used "redundantly" is for HTMLHyperlinkElementUtils, the other cases are on attributes on interfaces to restrict the exposure set.

Until we change the spec to always specify Exposed on interfaces, I think omitting it is better for consistency, and I'll remove it from HTMLHyperlinkElementUtils to avoid confusion.

@PoojaSanklecha
Copy link
Contributor Author

Ok. I'll remove it.

@PoojaSanklecha
Copy link
Contributor Author

Dropped [Exposed = Window] for DOMStringMap

@PoojaSanklecha
Copy link
Contributor Author

I have added link to bug in the last commit message and added my name to the acknowledgement !

@foolip
Copy link
Member

foolip commented Oct 30, 2015

Thanks! I have squashed the commits into a single one and pushed as 1b4b0ef. Going forward, it's helpful if you have only one commit, updating it with git commit --amend if changes are required.

@foolip foolip closed this Oct 30, 2015
foolip added a commit that referenced this pull request Oct 30, 2015
@PoojaSanklecha
Copy link
Contributor Author

Sure. Will keep in mind.

@annevk
Copy link
Member

annevk commented Oct 30, 2015

Thank you @PoojaSanklecha!

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

Successfully merging this pull request may close these issues.

3 participants