Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(424)

Issue 6741065: Derive DOM element tamings from specification data. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 4 months ago by kpreid2
Modified:
11 years, 4 months ago
Reviewers:
Jasvir
CC:
google-caja-discuss_googlegroups.com
Base URL:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Instead of Domado listing elements which get specialized taming classes, the HTML schema now lists every HTML element's DOM interface name, and Domado selects the implementation based on that name. The schema data is from the current version of the HTML5 spec at <http://dev.w3.org/html5/spec/section-index.html#elements-1>. Adds a warning whenever we *don't* have a taming class for a standard DOM interface; adds trivial classes for all empty DOM interfaces to suppress that warning. @r5152

Patch Set 1 #

Patch Set 2 : Derive DOM element tamings from specification data. #

Patch Set 3 : Derive DOM element tamings from specification data. #

Total comments: 8

Patch Set 4 : Derive DOM element tamings from specification data. #

Total comments: 2

Patch Set 5 : Derive DOM element tamings from specification data. #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+537 lines, -321 lines) Patch
M src/com/google/caja/lang/html/HTML.java View 1 2 3 4 2 chunks +11 lines, -1 line 0 comments Download
M src/com/google/caja/lang/html/HtmlDefinitions.java View 1 2 3 4 4 chunks +56 lines, -16 lines 0 comments Download
M src/com/google/caja/lang/html/HtmlSchema.java View 1 2 3 4 3 chunks +7 lines, -2 lines 0 comments Download
M src/com/google/caja/lang/html/html4-elements-defs.json View 1 2 3 4 1 chunk +182 lines, -91 lines 0 comments Download
M src/com/google/caja/lang/html/html5-elements-defs.json View 1 2 3 4 1 chunk +60 lines, -30 lines 0 comments Download
M src/com/google/caja/plugin/domado.js View 1 2 3 4 24 chunks +183 lines, -163 lines 0 comments Download
M src/com/google/caja/plugin/html-schema.js View 1 2 3 4 6 chunks +37 lines, -17 lines 4 comments Download
M tests/com/google/caja/plugin/JQueryTest.java View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9
kpreid2
11 years, 4 months ago (2012-10-23 20:06:06 UTC) #1
kpreid2
Updated to fix ES5/3 compatibility due to a freeze ordering problem.
11 years, 4 months ago (2012-10-23 21:45:16 UTC) #2
Jasvir
LGTM https://codereview.appspot.com/6741065/diff/6001/src/com/google/caja/lang/html/html4-elements-defs.json File src/com/google/caja/lang/html/html4-elements-defs.json (right): https://codereview.appspot.com/6741065/diff/6001/src/com/google/caja/lang/html/html4-elements-defs.json#newcode342 src/com/google/caja/lang/html/html4-elements-defs.json:342: "interface": "HTMLElement" }, HTMLIsIndexElement? https://codereview.appspot.com/6741065/diff/6001/src/com/google/caja/lang/html/html4-elements-defs.json#newcode385 src/com/google/caja/lang/html/html4-elements-defs.json:385: "interface": "HTMLMetaElement" ...
11 years, 4 months ago (2012-11-06 04:52:25 UTC) #3
kpreid2
Added HTMLUnknownElement per spec; please review that aspect of the code. No changes to schema ...
11 years, 4 months ago (2012-11-06 19:19:54 UTC) #4
Jasvir
For fun I ran a check on Chrome using these html4 and html5 defs. xxx.forEach(function(x) ...
11 years, 4 months ago (2012-11-09 22:35:32 UTC) #5
Jasvir
https://codereview.appspot.com/6741065/diff/11001/src/com/google/caja/plugin/domado.js File src/com/google/caja/plugin/domado.js (right): https://codereview.appspot.com/6741065/diff/11001/src/com/google/caja/plugin/domado.js#newcode5900 src/com/google/caja/plugin/domado.js:5900: 'HTMLTableCellElement', So should this become HTMLTableDataCell now? How does ...
11 years, 4 months ago (2012-11-09 23:08:54 UTC) #6
kpreid2
New snapshot. https://codereview.appspot.com/6741065/diff/11001/src/com/google/caja/plugin/domado.js File src/com/google/caja/plugin/domado.js (right): https://codereview.appspot.com/6741065/diff/11001/src/com/google/caja/plugin/domado.js#newcode5900 src/com/google/caja/plugin/domado.js:5900: 'HTMLTableCellElement', On 2012/11/09 23:08:54, Jasvir wrote: > ...
11 years, 4 months ago (2012-11-12 20:29:10 UTC) #7
Jasvir
With the caveat to doublecheck the use of gOPN, LGTM https://codereview.appspot.com/6741065/diff/14001/src/com/google/caja/plugin/html-schema.js File src/com/google/caja/plugin/html-schema.js (right): https://codereview.appspot.com/6741065/diff/14001/src/com/google/caja/plugin/html-schema.js#newcode176 ...
11 years, 4 months ago (2012-11-13 17:58:08 UTC) #8
kpreid2
11 years, 4 months ago (2012-11-13 18:14:09 UTC) #9
https://codereview.appspot.com/6741065/diff/14001/src/com/google/caja/plugin/...
File src/com/google/caja/plugin/html-schema.js (right):

https://codereview.appspot.com/6741065/diff/14001/src/com/google/caja/plugin/...
src/com/google/caja/plugin/html-schema.js:176: table[ELEMENT_DOM_INTERFACES[el]]
= true;
On 2012/11/13 17:58:08, Jasvir wrote:
> Note - it's safe to use object as a dictionary here since
> ELEMENT_DOM_INTERFACES is known not to contain elements
> like toString and valueOf.

I'm not sure which object (ELEMENT_DOM_INTERFACES or table) you're referring to
here. Either way, this is safe even if that is not the case: we iterate over own
properties of ELEMENT_DOM_INTERFACES, and the only thing we ever do with 'table'
is get its own properties. This is just a deduplicator.

https://codereview.appspot.com/6741065/diff/14001/src/com/google/caja/plugin/...
src/com/google/caja/plugin/html-schema.js:179: scriptInterfacesCache =
cajaVM.def(Object.getOwnPropertyNames(table));
On 2012/11/13 17:58:08, Jasvir wrote:
> Double check - this file gets run cajoled (in which case
> Object.getOwnPropertyNames is shimmed in) or in SES
> in which case gOPN is browser provided).

Yes. This is part of the bundle which includes Domado. Note that it uses
cajaVM.def which only exists in Caja environments, regardless of browser ES5
support.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b