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

Browser compat for <html>, <base>, <head>, <link>, <style>, <title>, and <meta> elements #279

Merged
merged 8 commits into from
Aug 2, 2017

Conversation

teoli2003
Copy link
Contributor

@teoli2003 teoli2003 commented Jul 3, 2017

Added compat data for the element (first html element with compat data).

A few points:

  • 5 commits, 'set-up for html' updates the travis/npm files so that they use html/. The other onres iare the compat data itself for , , , and .
  • Structure is html/elements/'element-name'[/'attribute-name'].
  • There will be html/global-attributes/'globla-attribute-name' in the future.
  • The MDN page is not yet updated. I will wait for a merge before going further ahead.

@Elchi3 Elchi3 added the data:html 📄 Compat data for HTML elements. https://developer.mozilla.org/docs/Web/HTML label Jul 4, 2017
@teoli2003 teoli2003 force-pushed the html-elements branch 3 times, most recently from 30812c0 to ff83b82 Compare July 4, 2017 16:29
@teoli2003 teoli2003 changed the title Browser compat for <html> element Browser compat for <html>, <base>, <head>, and <link> elements Jul 4, 2017
@teoli2003 teoli2003 changed the title Browser compat for <html>, <base>, <head>, and <link> elements Browser compat for <html>, <base>, <head>, <link>, and <style> elements Jul 5, 2017
@teoli2003 teoli2003 changed the title Browser compat for <html>, <base>, <head>, <link>, and <style> elements Browser compat for <html>, <base>, <head>, <link>, <style>, and <title> elements Jul 5, 2017
@teoli2003 teoli2003 changed the title Browser compat for <html>, <base>, <head>, <link>, <style>, and <title> elements Browser compat for <html>, <base>, <head>, <link>, <style>, <title>, and <meta> elements Jul 5, 2017
@Elchi3 Elchi3 self-requested a review July 6, 2017 16:50
@teoli2003 teoli2003 force-pushed the html-elements branch 2 times, most recently from 8534a07 to 83b4aab Compare July 7, 2017 09:08
@teoli2003
Copy link
Contributor Author

I've rebased the PR (it was needed because of PR #276)

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

A few things for you to fix.
I'm still thinking about the general structure of this. Doing a few experiments with the macro to see what's best.

},
"ie": {
"version_added": true,
"notes": "Before Internet Explorer 7, <base> could be positioned anywhere in the document and the nearest value of <base> was used."
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be <code>&lt;base&gt;</code> (2 times).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed (and put this in the present tense). Will be in the next rebased PR.

@@ -11,7 +11,7 @@ JSON file containing the compatibility data.

- [api/](https://github.com/mdn/browser-compat-data/tree/master/api) contains data for each [Web API](https://developer.mozilla.org/en-US/docs/Web/API) interface.

- [css/](https://github.com/mdn/browser-compat-data/tree/master/css) contains data for [CSS](https://developer.mozilla.org/en-US/docs/Web/CSS) properties, selectors and at-rules.
- [html/](https://github.com/mdn/browser-compat-data/tree/master/html) contains data for [HTML](https://developer.mozilla.org/en-US/docs/Web/HTML) elements, attributes and global attributes.
Copy link
Member

Choose a reason for hiding this comment

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

Don't replace the CSS sentence, add the HTML one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. (Once I upgrade the PR of course)

}
},
"relative_url": {
"desc": "Support of relative URIs.",
Copy link
Member

Choose a reason for hiding this comment

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

"Support of" is unnecessary. Let's keep this concise: "Relative URIs" and the key could be "relative_uris", too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Will be in the next rebased PR.

"deprecated": false
}
},
"alternate stylesheet": {
Copy link
Member

Choose a reason for hiding this comment

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

"alternate_stylesheets" as a key and desc: "alternate stylesheets" ? Unsure if I like space in feature keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Will be in the next rebased PR. I put desc: "Alternative stylesheets."

},
"firefox": {
"version_added": "39",
"notes": "Before Firefox 41, it didn't obey the crossorigin attribute."
Copy link
Member

Choose a reason for hiding this comment

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

Lets put crossorigin in <code> and use present tense.
"notes": "Prior to Firefox 41, the <code>crossorigin</code> attribute isn't obeyed."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. (Once I upgrade the PR of course)

"deprecated": false
}
},
"name=referer": {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be a sub feature of name?

Copy link
Contributor Author

@teoli2003 teoli2003 Jul 18, 2017

Choose a reason for hiding this comment

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

It already was (and is).

Copy link
Contributor Author

@teoli2003 teoli2003 Jul 18, 2017

Choose a reason for hiding this comment

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

@Elchi3 Maybe you want me to rename it in "referer" or "referer value"…

"version_removed": "35.0",
"flag": {
"type": "preference",
"name": "Enable <style scoped>",
Copy link
Member

Choose a reason for hiding this comment

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

"name": "Enable &lt;style scoped&gt;" (we probably need to fix the KS macro to html escape everything).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Will be in the next rebased PR.

Note that you can't automate escaping as <code> and <a> are ambiguous.

@Elchi3
Copy link
Member

Elchi3 commented Jul 10, 2017

Besides the comments above, I think there is agreement that your original structure (structure 1 in issue #283) is the one we should use.

@teoli2003
Copy link
Contributor Author

teoli2003 commented Jul 18, 2017

@Elchi3 I've updated the PR with all the changes requested. Ready for a new review.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thanks, lets take this in and I will try to convert it to the new format using my conversion script. It will be another good set for testing. But please be on hold for more migration for now, though. It is enough HTML test data for the moment.

@Elchi3 Elchi3 merged commit c30cc64 into mdn:master Aug 2, 2017
foolip added a commit to foolip/browser-compat-data that referenced this pull request Dec 19, 2022
Inconsistency pointed out by @gsnedders here:
mdn#18368

Most of this is originally from wiki migration:
mdn#279

The Chromium implementation was in M34:
https://chromium.googlesource.com/chromium/src/+/21e65a63bf8e3947ff4b4d34cb73866c413106e2
https://www.chromium.org/developers/calendar/

There was no use of "crossorigin" in HTMLLinkElement.cpp before this
point, and https://crbug.com/178787 doesn't say anything to suggest
there was partial support, so assume the old data was wrong.

For Edge and Safari, just trust the data for HTMLLinkElement.
Elchi3 added a commit that referenced this pull request Dec 21, 2022
* Update <link crossorigin> data to match API

Inconsistency pointed out by @gsnedders here:
#18368

Most of this is originally from wiki migration:
#279

The Chromium implementation was in M34:
https://chromium.googlesource.com/chromium/src/+/21e65a63bf8e3947ff4b4d34cb73866c413106e2
https://www.chromium.org/developers/calendar/

There was no use of "crossorigin" in HTMLLinkElement.cpp before this
point, and https://crbug.com/178787 doesn't say anything to suggest
there was partial support, so assume the old data was wrong.

For Edge and Safari, just trust the data for HTMLLinkElement.

* Update link.json

* Update html/elements/link.json

Co-authored-by: Florian Scholz <fs@florianscholz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:html 📄 Compat data for HTML elements. https://developer.mozilla.org/docs/Web/HTML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants