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

Add `intrinsicsize` to HTML Media elements (img, video) #2979

Merged
merged 5 commits into from Nov 8, 2018

Conversation

Projects
None yet
4 participants
@ZaneHannanAU
Copy link
Contributor

ZaneHannanAU commented Oct 12, 2018

Experimental intrinsicsize attribute added.
Explainer: https://github.com/ojanvafai/intrinsicsize-attribute

Chrome: Experimental Web Platform features flag, Version >= Chrome 71. https://www.chromestatus.com/feature/4704436815396864

Tested: no
Primarily done through manual JSON.parse and JSON.stringify ... may need changes for standard_track but not certain.

ZaneHannanAU added some commits Oct 12, 2018

Add `intrinsicsize`
Experimental `intrinsicsize` attribute added.
Explainer: https://github.com/ojanvafai/intrinsicsize-attribute

Chrome: `Experimental Web Platform features` flag, Version >= Chrome 71. https://www.chromestatus.com/feature/4704436815396864
Firefox: No public signals
Edge: No public signals
Safari: No public signals
Web Developers: No signals
@ZaneHannanAU

This comment has been minimized.

Copy link
Contributor

ZaneHannanAU commented Oct 12, 2018

Oh yeah need to add browser versions...

@ZaneHannanAU

This comment has been minimized.

Copy link
Contributor

ZaneHannanAU commented Oct 12, 2018

... can wait until full release.

@Elchi3 Elchi3 requested a review from wbamberg Oct 15, 2018

@wbamberg
Copy link
Member

wbamberg left a comment

Thanks for your contribution!

... can wait until full release.

It's up to you of course, but I think we can update the Chrome browser list to include 71 now, since our list is out of date already (https://github.com/mdn/browser-compat-data/blob/master/browsers/chrome_android.json#L229-L234). (Is 71 the current Nightly? I'm not sure how things like Canary map to the definitions in browsers.json.) That would unblock this PR.

But it would probably be good to update the list of browsers in a separate PR.

A few comments on this PR:

"flags": [
  {
    "type": "preference",
    "name": "Experimental Web Platform Features"
  }
]

See for example https://github.com/mdn/browser-compat-data/blob/master/api/Request.json#L538-L545.

@ZaneHannanAU

This comment has been minimized.

Copy link
Contributor

ZaneHannanAU commented Oct 17, 2018

Understood, likely will do this evening.

@jpmedley

This comment has been minimized.

Copy link
Contributor

jpmedley commented Oct 17, 2018

Two points. First, always use the anchor form rather than the string form of the flag. The explanation is here. @Elchi3 will be adding it to the linter.

Second, this is behind the #enable-experimental-productivity-features flag. On the commit expand the changes to the svg_image_element.idl and look for RuntimeEnabled=ExperimentalProductivityFeatures.

Show resolved Hide resolved html/elements/img.json Outdated

Elchi3 added some commits Nov 8, 2018

@Elchi3

Elchi3 approved these changes Nov 8, 2018

Copy link
Member

Elchi3 left a comment

I've marked this as non-standard, inserted the features alphabetically, and fixed the flag information. Looks good to me now.

All comments addressed, thanks for your review!

@Elchi3 Elchi3 merged commit 5061bf5 into mdn:master Nov 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ZaneHannanAU

This comment has been minimized.

Copy link
Contributor

ZaneHannanAU commented Nov 8, 2018

Thank you, sorry about … completely forgetting

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