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

React 16 does not lowercase HTML attributes in generated HTML #10863

Closed
chromakode opened this issue Sep 27, 2017 · 11 comments
Closed

React 16 does not lowercase HTML attributes in generated HTML #10863

chromakode opened this issue Sep 27, 2017 · 11 comments

Comments

@chromakode
Copy link

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

ReactDOMServer generates camelCased markup for the cellSpacing and cellPadding attributes:

<table cellSpacing="1" cellPadding="2"></table>

(Here's an example pen: https://codepen.io/anon/pen/jGBLdP)

I believe these attributes are canonically lowercased. If I lowercase the attributes in JSX, React warns that I'm not using the right names:

Warning: Invalid DOM property `cellpadding`. Did you mean `cellPadding`?

What is the expected behavior?

The attribute names would be rendered lowercase:

<table cellspacing="1" cellpadding="2"></table>

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

This is in 16.0.0. Prior versions of React stripped these attributes.

@thysultan
Copy link

thysultan commented Sep 27, 2017

Since HTML attributes are case-insensitive this should work as expected.

var div = document.createElement('div')
div.innerHTML = '<table cellSpacing="1" cellPadding="2"></table>'
// produces
<table cellspacing="1" cellpadding="2"></table>
div.cellPadding === "2" // true

@chromakode
Copy link
Author

Yes, agreed. Though if there is an easy way to make React output these attributes lower-cased, that would be a more canonical way to format these (does it currently do this for any other attributes?)

@gaearon
Copy link
Collaborator

gaearon commented Sep 27, 2017

Seems like it doesn't matter in practice. Although I'm open to reviewing PRs that lowercase anything that is in HTML namespace.

@gaearon gaearon changed the title React 16: cellSpacing and cellPadding attributes camelCased in generated HTML React 16 does not lowercase HTML attributes in generated HTML Oct 3, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

I'm tagging this as feature request because technically it's not a bug. But we can fix this.

@gaearon
Copy link
Collaborator

gaearon commented Jan 5, 2018

I decided not to fix this. Again, it's not a bug: technically HTML allows attributes to have any casing. We're strictly doing less work on React side, and it doesn't make sense to me to do more work to make the output more aesthetically consistent.

If you feel strongly about you're welcome to provide real-world benchmarks from sites heavily using SSR that demonstrate #11110 has no impact on perf. Then we could take it.

@namnm
Copy link

namnm commented Jun 18, 2018

Hey guys, not sure why but charSet makes the page shows invalid character code in some browsers (on iPhone), change it to all lowercase as charset and it works as normal. Now I must render them all to string and use replace to avoid this case.

@gaearon
Copy link
Collaborator

gaearon commented Jun 18, 2018

Can you file a new issue for this specifically with a reproducing case? Then we could look at it.
Thank you!

@cubuspl42
Copy link

I believe that a strong argument in favour of lowercasing (normalising) attributes is that W3 validator marks cammel-cased attributes as an error.

Line 1, Column 584: there is no attribute "cellSpacing"

image

While the standard might allow aRbiTrAryCasinG, W3 validator suggests that there's such thing as a normalised attribute name and that not using it can be considered an error.

@cubuspl42
Copy link

By the way, the funny thing is that React actually complains if one tries to override the attribute name to make the React output validate in the W3 validator:

<table
  style={props.style}
  {...{
    cellspacing: "0",
    cellpadding: "0",
  }}
> ...
Warning: Invalid DOM property `cellpadding`. Did you mean `cellPadding`?
    at table

@antsukanova
Copy link

antsukanova commented Jan 21, 2022

I believe that a strong argument in favour of lowercasing (normalising) attributes is that W3 validator marks cammel-cased attributes as an error.

Line 1, Column 584: there is no attribute "cellSpacing"

image

While the standard might allow aRbiTrAryCasinG, W3C validator suggests that there's such thing as a normalized attribute name and that not using it can be considered an error.

Usecase: I want to render specific HTML strict markup for emails using React. W3 validator says the same thing as on the screenshot above.
If even this error in HTML doesn't affect users there are developers who rely on this validation and even have it as a requirement in their tasks. So now you can’t just create an email template generator using React with ReactDOMServer because it can't produce code without errors in w3c.

@reverofevil
Copy link

@gaearon So we figured out it's actually a bug. Should this issue be reopened, or another one created?

gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 18, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparision, hammerhead will be more
accepting of inputs and resolve srcset issues with React projets (DevExpress#2958).

See facebook/react#10863 for more background.
gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 18, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparision, hammerhead will be more
accepting of inputs and resolve srcset issues with React projets
(fixes DevExpress#2958).

See facebook/react#10863 for more background.
gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 18, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparision, hammerhead will be more
accepting of inputs and resolve srcset issues with React projets
(fixes DevExpress#2958).

See facebook/react#10863 for more background.
gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 18, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparison, hammerhead will be more
accepting of inputs and resolve srcset issues with React projects
(fixes DevExpress#2958).

See facebook/react#10863 for more background.
gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 18, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparison, hammerhead will be more
accepting of inputs and resolve srcset issues with React projects
(fixes DevExpress#2958).

See facebook/react#10863 for more background.
gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 18, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparison, hammerhead will be more
accepting of inputs and resolve srcset issues with React projects
(fixes DevExpress#2958).

See facebook/react#10863 for more background.
gg-kialo added a commit to gg-kialo/testcafe-hammerhead that referenced this issue Oct 24, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparison, hammerhead will be more
accepting of inputs and resolve srcset issues with React projects
(fixes DevExpress#2958).

See facebook/react#10863 for more background.
Aleksey28 pushed a commit to DevExpress/testcafe-hammerhead that referenced this issue Oct 24, 2023
Some frameworks, like React, use non-canonicalised attribute names. The
spec and browsers allow this, but hammerhead is only checking if the
attribute name without canonicalisation matches `srcset`.

By using the `loweredAttr` for the comparison, hammerhead will be more
accepting of inputs and resolve srcset issues with React projects
(fixes #2958).

See facebook/react#10863 for more background.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants
@chromakode @gaearon @thysultan @cubuspl42 @reverofevil @namnm @antsukanova and others