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

<head profile="...">: profile attribute is not supported #6029

Closed
saiichihashimoto opened this issue Feb 12, 2016 · 9 comments
Closed

<head profile="...">: profile attribute is not supported #6029

saiichihashimoto opened this issue Feb 12, 2016 · 9 comments
Labels
Difficulty: starter

Comments

@saiichihashimoto
Copy link
Contributor

https://facebook.github.io/react/docs/tags-and-attributes.html : profile is missing
https://facebook.github.io/react/docs/jsx-gotchas.html : If you pass properties to native HTML elements that do not exist in the HTML specification, React will not render them.

Meanwhile, profile attributes is in standards (kind of): http://www.w3schools.com/tags/tag_head.asp.

^ Above is a blatant copy of #4226 but hey, that worked out.

Also, anticipating the need to glance at the Mozilla Developer site: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/head#attr-profile

One day, #140 will be closed and this won't be an issue anymore. :-)
@saiichihashimoto saiichihashimoto mentioned this issue Feb 12, 2016
Closed
@zpao
Copy link
Member

zpao commented Feb 12, 2016

I guess obsoleted in HTML5 is fine. Would you like to add it?

@saiichihashimoto
Copy link
Contributor Author

Embarrassingly, I don't know a whole lot about what I'm doing here (though I do like learning!). It seems like it would be pretty straightforward to copy the change that solved #4226, but I just want to double check that that is all that would be required...?

@saiichihashimoto
Copy link
Contributor Author

In terms of being obsoleted, it still seems to be in use for opensearch so I'm imagining its useful to have around.

@zpao
Copy link
Member

zpao commented Feb 12, 2016

Pretty much. The important thing to check is what updating the value would look like (even if your case is server rendering). My common way to test this is to load up a data uri and then manipulate the DOM via JS to set the property/attribute and then see what works.

  1. Visit data:text/html,<!doctype html><head profile="foo">
  2. Open console
  3. use
var h = document.getElementsByTagName('head')[0]
h.profile // undefined
h.getAttribute('profile') // foo
h.setAttribute('profile', 'bar') // dom is updated in inspector
// can do the same with reading/writing h.profile but see that it doesn't work

So the result of that is that MUST_USE_ATTRIBUTE is right and that your diff ends up being pretty identical. You can leave out the docs update though - we try to batch those changes up at release time.

@saiichihashimoto
Copy link
Contributor Author

I'm confident you know what you're talking about but I'm noticing that, currently, the wrap property is null instead of MUST_USE_ATTRIBUTE. Again, I'm admitting ignorance and am not sure what that value means. I've tracked it down to this file but, even with that, I'm lost as to what it does. I'm going to have to ask you @zpao to hold my hand through this a LITTLE longer. :-)

@jimfb
Copy link
Contributor

jimfb commented Feb 12, 2016

@saiichihashimoto We recently changed from using properties to using attributes by default for a whole variety of reasons. You can learn more about properties vs. attributes here: http://lucybain.com/blog/2014/attribute-vs-property/

Most values can be updated via attributes, so you can set the value to null. Some html elements must be updated using the property instead of the attribute (otherwise, the changes won't take effect by the browser), in which case, the value must be MUST_USE_PROPERTY. You should verify in browsers (especially something like IE9, which tends to behave weirdly) to verify that the browser is honoring the new value.

In this case, I'm not entirely sure if the browser even does anything with the profile attribute, does it? Does the presence (or lack thereof) actually influence browser behavior at all? If not, then using attributes is certainly fine.

@saiichihashimoto
Copy link
Contributor Author

@jimfb I haven't tested specifically if browsers actually use it. However, according to the opensearch specs:

The HTML element should include a "profile" attribute that contains the value "http://a9.com/-/spec/opensearch/1.1/".

If its part of their specs then opensearch clients (now or in the future) very well may check for it.

@jimfb
Copy link
Contributor

jimfb commented Feb 12, 2016

@saiichihashimoto Right, but if browsers don't actually use it, then it's almost certainly sufficient to set it as an attribute and not a property. Therefore, to answer your question, null is probably the most appropriate value.

@zpao
Copy link
Member

zpao commented Feb 13, 2016

Ah, my mistake. I forgot we switched to attributes by default. As I mentioned before with my test "code", the property doesn't work so we need to use attributes.

@saiichihashimoto saiichihashimoto mentioned this issue Feb 13, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Difficulty: starter
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@zpao @saiichihashimoto @jimfb