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

IDL attribute reflection branch #708

Merged
merged 8 commits into from
May 22, 2018
Merged

IDL attribute reflection branch #708

merged 8 commits into from
May 22, 2018

Conversation

cookiecrook
Copy link
Contributor

@cookiecrook cookiecrook commented Apr 2, 2018

Closes #691


Preview | Diff

@cookiecrook cookiecrook requested a review from joanmarie April 2, 2018 06:03
@cookiecrook
Copy link
Contributor Author

cookiecrook commented Apr 2, 2018

I have now linked my GitHub and W3C accounts, but I do not have privs to revalidate the failing IPR check.

@ZoeBijl
Copy link

ZoeBijl commented Apr 2, 2018

I see a bunch of links that point to the WHATWG HTML spec. Are these pages unavailable in the W3C HTML spec?

index.html Outdated
<tr><td><dfn>ariaLive</dfn></td><td><pref>aria-live</pref></td></tr>
<tr><td><dfn>ariaModal</dfn></td><td><pref>aria-modal</pref></td></tr>
<tr><td><dfn>ariaMultiLine</dfn></td><td><pref>aria-multiline</pref></td></tr>
<tr><td><dfn>ariaMultiSelectable</dfn></td><td><pref>aria-selectable</pref></td></tr>
Copy link

Choose a reason for hiding this comment

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

should this be aria-multiselectable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes thanks

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Apr 2, 2018

Zoe, if you post review comments on the specific line, it will be easier to track this down. I found one link (repeated twice) to the Reflection definition. Is this what you meant by "a bunch"? I linked the WHATWG version b/c the W3C spec is frequently out-of-date, but for the purposes of this reference, the W3C version will do. Added commit c424047.

@ZoeBijl
Copy link

ZoeBijl commented Apr 12, 2018

@cookiecrook sorry for not being clearer before—will add line comments next time. My use of “a bunch” was incorrect, just tried to cover my butt in case I missed one 😬.

Thanks for fixing it.

…gs), and DOMTokenLists can't be both reaonly and reflected (token list reflection will be handled differently).
…enList doesn't gain authors much and makes reflection more difficult.
@minorninth
Copy link

Can you clarify why [Reflect] was removed?

Is it because the semantics were wrong, or is it because the IDL syntax doesn't officially support reflecting attributes whose property names don't match?

@cookiecrook
Copy link
Contributor Author

[Reflect] was removed because it's non-standard IDL, but FWIW, the WebKit and Chrome engines are both likely to use the previous iteration of this PR that included reflection attribute names.

@minorninth
Copy link

Thanks. I see now that this more or less matches how other specs are written. [Reflect] isn't part of the official WebIDL spec, so the definition of each attribute explicitly indicates that it reflects a content attribute.

@jnurthen jnurthen merged commit c040ddc into master May 22, 2018
joanmarie pushed a commit that referenced this pull request Jun 28, 2018
@jnurthen jnurthen deleted the attr-reflection branch July 23, 2020 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants