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

Implement SupportedPropertyNames methods #7273

Closed
frewsxcv opened this issue Aug 18, 2015 · 2 comments
Closed

Implement SupportedPropertyNames methods #7273

frewsxcv opened this issue Aug 18, 2015 · 2 comments
Assignees
Labels

Comments

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Aug 18, 2015

#7254 introduces a method on some DOM types called SupportedPropertyNames that returns all the 'supported property names' as per their respective interface. That PR only implements HTMLCollectionMethods::SupportedPropertyNames (I didn't have enough time to complete the rest). The other ones need to be implemented.

@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Mar 11, 2016

At the time of writing, there is one impl remaining to be implemented:

// https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
fn SupportedPropertyNames(&self) -> Vec<DOMString> {
// FIXME: unimplemented (https://github.com/servo/servo/issues/7273)
vec![]
}

@pshaughn
Copy link
Member

@pshaughn pshaughn commented Jan 16, 2020

A few years later, the above comment continues to be effectively true. (Plugin and PluginArray also have stubs, but that's correct for a plugin-less world.)

@pshaughn pshaughn self-assigned this Jan 17, 2020
bors-servo added a commit that referenced this issue Jan 22, 2020
Add SupportedPropertyNames to Document (also fix iframe getting)

Existing test of named-getting an iframe now succeeds. I added a new test for Object.getOwnPropertyNames(document) based on my understanding of the spec; that test could use a second opinion.

UPDATE: This was trying to do too many things in one PR as originally submitted. It is now using #25572 as a base, and I suggest reviewing that PR before this one to avoid duplicating review effort.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7273 for all implemented named getters, fix #25146, and fix the iframe case only of #25145.

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 13, 2020
Add SupportedPropertyNames to Document (also fix iframe getting)

Existing test of named-getting an iframe now succeeds. I added a new test for Object.getOwnPropertyNames(document) based on my understanding of the spec; that test could use a second opinion.

UPDATE: This was trying to do too many things in one PR as originally submitted. It is now using #25572 as a base, and I suggest reviewing that PR before this one to avoid duplicating review effort.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7273 for all implemented named getters, fix #25146, and fix the iframe case only of #25145.

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
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.

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