Issue 5529044: code review 5529044: html: foreign element HTML integration points, tag name...

Issue 5529044: code review 5529044: html: foreign element HTML integration points, tag name... (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
13 years, 6 months ago by nigeltao

Modified:
13 years, 6 months ago

Reviewers:

CC:
andybalholm, golang-dev

Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : diff -r 52ae6fbcc97a https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 52ae6fbcc97a https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 4 : diff -r 3cd651bb0d3a https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 5 : diff -r 7532f9705801 https://go.googlecode.com/hg/ #

Created: 13 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+196 lines, -91 lines)			Patch
M	src/pkg/html/foreign.go	View	1	2 chunks	+58 lines, -1 line	0 comments	Download
M	src/pkg/html/node.go	View	1	1 chunk	+4 lines, -0 lines	0 comments	Download
M	src/pkg/html/parse.go	View	1 2 3	26 chunks	+133 lines, -89 lines	0 comments	Download
M	src/pkg/html/parse_test.go	View	1	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 6

Expand All Messages | Collapse All Messages

nigeltao

Hello andybalholm@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/

13 years, 6 months ago (2012-01-07 06:35:45 UTC) #1

andybalholm

http://codereview.appspot.com/5529044/diff/4001/src/pkg/html/parse.go File src/pkg/html/parse.go (right): http://codereview.appspot.com/5529044/diff/4001/src/pkg/html/parse.go#newcode104 src/pkg/html/parse.go:104: func (p *parser) indexOfElementInScope(useDefaultStopTags bool, extraStopTags []string, matchTags ...string) ...

13 years, 6 months ago (2012-01-09 17:03:32 UTC) #2

nigeltao

13 years, 6 months ago (2012-01-09 23:44:54 UTC) #3

andybalholm

LGTM. http://codereview.appspot.com/5529044/diff/7001/src/pkg/html/parse.go File src/pkg/html/parse.go (right): http://codereview.appspot.com/5529044/diff/7001/src/pkg/html/parse.go#newcode70 src/pkg/html/parse.go:70: tableRowScope Should you document that some scopes can ...

13 years, 6 months ago (2012-01-09 23:56:59 UTC) #4

nigeltao

http://codereview.appspot.com/5529044/diff/7001/src/pkg/html/parse.go File src/pkg/html/parse.go (right): http://codereview.appspot.com/5529044/diff/7001/src/pkg/html/parse.go#newcode70 src/pkg/html/parse.go:70: tableRowScope On 2012/01/09 23:56:59, andybalholm wrote: > Should you ...

13 years, 6 months ago (2012-01-10 00:03:28 UTC) #5

nigeltao

13 years, 6 months ago (2012-01-10 00:06:16 UTC) #6

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

html: foreign element HTML integration points, tag name adjustment,
shorten the MathML namespace abbreviation from "mathml" to "math".
Python's html5lib uses "mathml", but I think that that is an internal
implementation detail; the test cases use "math".

Pass tests10.dat, test 30:
<div><svg><path><foreignObject><math></div>a

| <html>
|   <head>
|   <body>
|     <div>
|       <svg svg>
|         <svg path>
|           <svg foreignObject>
|             <math math>
|               "a"

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

Expand All Messages | Collapse All Messages