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

fix(next/image): render valid html according to W3C #33825

Merged
merged 11 commits into from Feb 1, 2022
2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -56,6 +56,7 @@
"@testing-library/react": "11.2.5",
"@types/cheerio": "0.22.16",
"@types/fs-extra": "8.1.0",
"@types/html-validator": "5.0.2",
"@types/http-proxy": "1.17.3",
"@types/jest": "24.0.13",
"@types/node": "13.11.0",
Expand Down Expand Up @@ -112,6 +113,7 @@
"get-port": "5.1.1",
"glob": "7.1.6",
"gzip-size": "5.1.1",
"html-validator": "6.0.0",
"image-size": "0.9.3",
"is-animated": "2.0.0",
"isomorphic-unfetch": "3.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/next/client/image.tsx
Expand Up @@ -606,7 +606,7 @@ export default function Image({
hasSizer = true
sizerStyle.maxWidth = '100%'
// url encoded svg is a little bit shorten than base64 encoding
sizerSvgUrl = `data:image/svg+xml,%3csvg xmlns=%27http://www.w3.org/2000/svg%27 version=%271.1%27 width=%27${widthInt}%27 height=%27${heightInt}%27/%3e`
sizerSvgUrl = `data:image/svg+xml,%3csvg+xmlns=%27http://www.w3.org/2000/svg%27+version=%271.1%27+width=%27${widthInt}%27+height=%27${heightInt}%27/%3e`
} else if (layout === 'fixed') {
// <Image src="i.png" width="100" height="100" layout="fixed" />
wrapperStyle.display = 'inline-block'
Expand Down
13 changes: 13 additions & 0 deletions test/integration/image-component/valid-w3c-html/pages/_document.js
@@ -0,0 +1,13 @@
import { Html, Head, Main, NextScript } from 'next/document'

export default function Document() {
return (
<Html lang="en">
<Head />
<body>
<Main />
<NextScript />
</body>
</Html>
)
}
18 changes: 18 additions & 0 deletions test/integration/image-component/valid-w3c-html/pages/index.js
@@ -0,0 +1,18 @@
import Head from 'next/head'
import Image from 'next/image'

export default function Home() {
return (
<div>
<Head>
<title>Create Next App</title>
<meta name="description" content="Generated by create next app" />
<link rel="icon" type="image/jpeg" href="/test.jpg" />
</Head>

<main>
<Image src="/test.jpg" alt="logo" width={200} height={200} />
</main>
</div>
)
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 42 additions & 0 deletions test/integration/image-component/valid-w3c-html/test/index.test.js
@@ -0,0 +1,42 @@
/* eslint-env jest */

import {
findPort,
killApp,
nextBuild,
nextStart,
waitFor,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import validateHTML from 'html-validator'
import { join } from 'path'

const appDir = join(__dirname, '../')
let appPort
let app
let browser

describe('Image Component Valid W3C HTML', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
browser = await webdriver(appPort, '/')
})
afterAll(() => {
killApp(app)
browser = null
})

it('should be vaild W3C HTML', async () => {
await waitFor(1000)
expect(await browser.hasElementByCssSelector('img')).toBeTruthy()
const url = await browser.url()
const result = await validateHTML({
Copy link
Member

Choose a reason for hiding this comment

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

We could add this to the existing image-component test suite here instead and ensure the different layouts all validate correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done! 👍

To keep things simple, I still added a new page /valid-html-w3c in the default folder.

url,
format: 'json',
isLocal: true,
})
expect(result.messages).toEqual([])
})
})