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

x/net/html: doesnt support attributes without values ie <script async "foo.js"> #40111

Open
Villenny opened this issue Jul 7, 2020 · 3 comments
Open

x/net/html: doesnt support attributes without values ie <script async "foo.js"> #40111

Villenny opened this issue Jul 7, 2020 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@Villenny
Copy link

Villenny commented Jul 7, 2020 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.14.4 windows/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\rhaksi\AppData\Local\go-build
set GOENV=C:\Users\rhaksi\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\rhaksi\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\dev\bitbucket\kidoz-team\bidder-server\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\rhaksi\AppData\Local\Temp\go-build383668822=/tmp/go-build -gno-record-gcc-switches

What did you do?

markup := `<!DOCTYPE html>
<html>
	<head>
		<title>.</title>
	</head>
	<body>
		<script async src="test.js"></script>
	</body>
</html>
`
r := strings.NewReader(markup)
doc, err := html.Parse(r)
if err != nil {
	return
}
var writer bytes.Buffer
err = html.Render(&writer, doc)
if err != nil {
	return
}
fmt.Println(writer.String())

What did you expect to see?

The async attribute should have no value.

<html>
	<head>
		<title>.</title>
	</head>
	<body>
		<script async src="test.js"></script>
	</body>
</html>

What did you see instead?

The async attribute is rendered with an extra ="" added to it.

<html>
	<head>
		<title>.</title>
	</head>
	<body>
		<script async="" src="test.js"></script>
	</body>
</html>

async is one of the 25 boolean attributes, it should be either present (value of true) or absent (value of false), and shouldnt have a value assigned.

The Boolean Attributes
allowfullscreen
allowpaymentrequest
async
autofocus
autoplay
checked
controls
default
disabled
formnovalidate
hidden
ismap
itemscope
loop
multiple
muted
nomodule
novalidate
open
playsinline
readonly
required
reversed
selected
truespeed
@gopherbot gopherbot added this to the Unreleased milestone Jul 7, 2020
@dmitshur
Copy link
Contributor

Aside from visual noise and extra bytes, does the added ="" cause any issues when the HTML is parsed?

/cc @mikioh @bradfitz @ianlancetaylor per owners.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 11, 2020
@Villenny
Copy link
Author

Villenny commented Jul 11, 2020 •
edited

For the one example I ran across when putting html through the parser and back, does it work in the latest version of chrome on windows for specifically the extra ="" being assigned to the async boolean attribute? It does seem to work, yes. At least in the sense that the html seems to render and the javascript seems to work correctly, although I cant say if the async attribute is still being obeyed (but probably)

On the other hand its an open question if it works in every version of every browser on every platform for every boolean attribute, even just the subset still commonly in use, because that I couldn't tell you. I suppose its possible. Its also of course impossible to make my minifier truly minify while using html.parse/render.

I imagine many html validators would fail the resulting output with an error for assigning a value to a boolean attribute. Although in my particular narrow use case this wont affect me.

Cheers

@namusyaka
Copy link
Member

namusyaka commented Oct 14, 2020 •
edited

As an other perspective, the current behavior is valid as per the whatwg spec:
https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes

Since most browsers respect the whatwg spec probably, the html code rendered by this package can be expected to work on browsers properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@namusyaka @dmitshur @Villenny @gopherbot