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.

Sign up for GitHub

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

Jump to bottom

Add HTMLOrSVGElement interface mixin #3543

Merged
merged 2 commits into from
Mar 23, 2018
Merged

Add HTMLOrSVGElement interface mixin #3543

merged 2 commits into from
Mar 23, 2018

Conversation

annevk
Copy link
Member

@annevk annevk commented Mar 7, 2018 •
edited by pr-preview bot
Loading

This exposes dataset, tabIndex, focus(), and blur() on SVG elements.

It also does away with NoncedElement.

Tests: ...

Fixes #3471.

/dom.html ( diff )
/infrastructure.html ( diff )
/urls-and-fetching.html ( diff )

@annevk
Add HTMLOrSVGElement interface mixin
b458861 
This exposes dataset, tabIndex, focus(), and blur() on SVG elements.

It also does away with NoncedElement.

Tests: ...

Fixes #3471.
@annevk annevk mentioned this pull request Mar 7, 2018
Closed
@dstorey dstorey mentioned this pull request Mar 7, 2018
Closed
@domenic domenic added addition/proposal New features or enhancements needs tests Moving the issue forward requires someone to write tests topic: focus labels Mar 7, 2018
domenic
domenic approved these changes Mar 7, 2018
View reviewed changes
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

At first glance the processing model appears to work fine for each of these.

What's the implementer interest status on all of these? Edge yes, anyone else? (Maybe for a subset?)

source Outdated
data-dfn-type="attribute">[[CryptographicNonce]]</dfn>, and exposing it to script via the
<code>NoncedElement</code> interface defined below. Unless otherwise specified, the slot's value
<code>HTMLOrSVGElement</code> mixin interface. Unless otherwise specified, the slot's value
Copy link
Member

Choose a reason for hiding this comment

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

mixin interface -> interface mixin

source Outdated
@@ -10195,6 +10186,21 @@ interface <dfn>HTMLUnknownElement</dfn> : <span>HTMLElement</span> { };</pre>

</div>

<p>Features shared between HTML and SVG elements use the <code>HTMLOrSVGElement</code> mixin
interface: <ref spec=SVG></p>
Copy link
Member

Choose a reason for hiding this comment

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

mixin interface -> interface mixin

@dstorey
Copy link
Member

dstorey commented Mar 7, 2018

@domenic All major browsers support tabindex/focus/blur/dataset on SVGElement https://developer.microsoft.com/en-us/microsoft-edge/platform/catalog/?page=1&q=svgel%20spec%3Adetected%20edge%3Adetected%20chrome%3Adetected%20firefox%3Adetected%20safari%3Adetected

@annevk
Copy link
Member Author

annevk commented Mar 7, 2018

@mikewest I suspect this might require downstream changes somewhere?

@dstorey dstorey mentioned this pull request Mar 9, 2018
Merged
@annevk
Copy link
Member Author

annevk commented Mar 20, 2018

It does not seem like CSP mentions NoncedElement so I think we're safe from that perspective.

I can update the web-platform-tests IDL. Any progress on tests @dstorey?

annevk added a commit to web-platform-tests/wpt that referenced this pull request Mar 20, 2018
@annevk
Add HTMLOrSVGElement to HTML's IDL
cafaab1 
And align a couple other things while there.

Helps with whatwg/html#3543.
@annevk annevk mentioned this pull request Mar 20, 2018
Merged
@dstorey
Copy link
Member

dstorey commented Mar 21, 2018

I should be done in the next day or two

@dstorey
Copy link
Member

dstorey commented Mar 23, 2018

@annevk I made my first attempt at tests at web-platform-tests/wpt#10149 I converted existing HTML tests to use the SVG file that SVG tests suppose to use, and adapted the focus flag test to use those defined in the scripting section of SVG.

@dstorey
Copy link
Member

dstorey commented Mar 23, 2018

note that dataset already has tests for it in SVG

@annevk
Copy link
Member Author

annevk commented Mar 23, 2018

Thanks, I think that's good enough to go ahead and merge this. I had one more question, should we consider doing this for more features? E.g., click() seemed like an obvious addition when I worked on this.

@dstorey
Copy link
Member

dstorey commented Mar 23, 2018

Perhaps. It will probably be easy to implement in Edge too. Might be something we want to discuss on the SVG call on Monday. We're already discussing changing SVGAElement members (outside of href and probably target) to the HTML definition; and if we do that those would probably make sense to move to HTMLorSVGAnchorElement or the existing mixin.

@annevk
Copy link
Member Author

annevk commented Mar 23, 2018

Okay, please open new issues for those if there's some agreement. In general I'm hugely in favor of increasing the number of code paths that are shared with SVG.

sideshowbarker pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 23, 2018
@annevk @sideshowbarker
Add HTMLOrSVGElement to HTML's IDL
20d739e 
And align a couple other things while there.

Helps with whatwg/html#3543.
@annevk annevk merged commit 81e8243 into master Mar 23, 2018
@annevk annevk deleted the annevk/htmlorsvgelement branch March 23, 2018 10:30
@dstorey dstorey mentioned this pull request Mar 23, 2018
Closed
@AmeliaBR AmeliaBR mentioned this pull request Mar 23, 2018
Closed
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 15, 2018
@annevk @jgraham
Bug 1447313 [wpt PR 10110] - Add HTMLOrSVGElement to HTML's IDL, a=te…
b50ca81 
…stonly

Automatic update from web-platform-testsAdd HTMLOrSVGElement to HTML's IDL

And align a couple other things while there.

Helps with whatwg/html#3543.

wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110
wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
@marco-c
Bug 1447313 [wpt PR 10110] - Add HTMLOrSVGElement to HTML's IDL, a=te…
af6053c 
…stonly

Automatic update from web-platform-testsAdd HTMLOrSVGElement to HTML's IDL

And align a couple other things while there.

Helps with whatwg/html#3543.

wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110
wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110

UltraBlame original commit: f9c70032691f8b6bc9cc1af98df0f5c4e84cd319
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
@marco-c
Bug 1447313 [wpt PR 10110] - Add HTMLOrSVGElement to HTML's IDL, a=te…
19a9a34 
…stonly

Automatic update from web-platform-testsAdd HTMLOrSVGElement to HTML's IDL

And align a couple other things while there.

Helps with whatwg/html#3543.

wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110
wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110

UltraBlame original commit: f9c70032691f8b6bc9cc1af98df0f5c4e84cd319
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
@marco-c
Bug 1447313 [wpt PR 10110] - Add HTMLOrSVGElement to HTML's IDL, a=te…
174b765 
…stonly

Automatic update from web-platform-testsAdd HTMLOrSVGElement to HTML's IDL

And align a couple other things while there.

Helps with whatwg/html#3543.

wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110
wpt-commits: 20d739eff6ebfe360c1aadfd3f3161c0fce847a2
wpt-pr: 10110

UltraBlame original commit: f9c70032691f8b6bc9cc1af98df0f5c4e84cd319
@bkardell bkardell mentioned this pull request Jul 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domenic domenic domenic approved these changes

Assignees
No one assigned
Labels
addition/proposal New features or enhancements needs tests Moving the issue forward requires someone to write tests topic: focus
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@annevk @dstorey @domenic