code review 5293051: html: parse <select> tags.

html: parse <select> tags.

The additional test case in parse_test.go is:
<select><b><option><select><option></b></select>X

[nigeltao, 2011-10-20 03:52:07]

Hello andybalholm@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[andybalholm, 2011-10-20 15:32:02]

It looks good except for how the inverted stop tags are handled.

http://codereview.appspot.com/5293051/diff/4001/src/pkg/html/parse.go
File src/pkg/html/parse.go (right):

http://codereview.appspot.com/5293051/diff/4001/src/pkg/html/parse.go#newcode53
src/pkg/html/parse.go:53: const invertStopTags = "\xff"
This is ugly.

Instead, we could replace

p.popUntil(selectScopeStopTags, "select")
return p.resetInsertionMode(), true

with

for i := len(p.oe) - 1; i >= 0; i-- {
	switch p.oe[i].Data {
	case "select":
		p.oe = p.oe[:i]
		return p.resetInsertionMode(), true
	case "option", "optgroup":
		continue
	default:
		return inSelectIM, true
	}
}

Or if you want to keep using popUntil, you could define a stopTags type with a
flag:

type stopTags struct {
    tags []string
    inverted bool
}

so that the flag won't be mixed in with the tag names.

[nigeltao, 2011-10-21 00:54:56]

PTAL.

http://codereview.appspot.com/5293051/diff/4001/src/pkg/html/parse.go
File src/pkg/html/parse.go (right):

http://codereview.appspot.com/5293051/diff/4001/src/pkg/html/parse.go#newcode53
src/pkg/html/parse.go:53: const invertStopTags = "\xff"
On 2011/10/20 15:32:02, andybalholm wrote:
> This is ugly.
> 
> Instead, we could replace
> 
> p.popUntil(selectScopeStopTags, "select")
> return p.resetInsertionMode(), true
> 
> with
> 
> for i := len(p.oe) - 1; i >= 0; i-- {
> 	switch p.oe[i].Data {
> 	case "select":
> 		p.oe = p.oe[:i]
> 		return p.resetInsertionMode(), true
> 	case "option", "optgroup":
> 		continue
> 	default:
> 		return inSelectIM, true
> 	}
> }

Done.

[andybalholm, 2011-10-21 21:39:35]

LGTM.

[nigeltao, 2011-10-22 09:18:20]

*** Submitted as http://code.google.com/p/go/source/detail?r=60d08510dc91 ***

html: parse <select> tags.

The additional test case in parse_test.go is:
<select><b><option><select><option></b></select>X

R=andybalholm
CC=golang-dev
http://codereview.appspot.com/5293051