Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1186)

Issue 9310103: Add a constructor for DOMParser. (Closed)

Created:
8 years, 10 months ago by nweiz
Modified:
8 years, 10 months ago
Reviewers:
podivilov, antonm, sra1
CC:
reviews_dartlang.org, dart-dom-team_google.com, Anton Muhin
Visibility:
Public.

Description

Add a constructor for DOMParser. Committed: https://code.google.com/p/dart/source/detail?r=3969

Patch Set 1 #

Total comments: 2

Patch Set 2 : Add tests and a template for DOMParser. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -5 lines) Patch
M client/dom/frog/dom_frog.dart View 3 chunks +8 lines, -1 line 0 comments Download
M client/dom/generated/src/frog/DOMParser.dart View 1 chunk +1 line, -0 lines 0 comments Download
M client/dom/generated/src/interface/DOMParser.dart View 1 chunk +3 lines, -1 line 0 comments Download
M client/dom/generated/wrapping_dom.js View 1 chunk +8 lines, -0 lines 0 comments Download
M client/dom/src/_FactoryProviders.dart View 1 chunk +7 lines, -0 lines 0 comments Download
M client/dom/src/frog_FactoryProviders.dart View 1 chunk +5 lines, -0 lines 0 comments Download
M client/dom/src/native_FactoryProviders.dart View 1 chunk +4 lines, -0 lines 0 comments Download
M client/dom/src/native_FactoryProvidersImplementation.dart View 1 chunk +1 line, -0 lines 0 comments Download
A + client/dom/templates/dom/interface/interface_DOMParser.darttemplate View 1 1 chunk +3 lines, -3 lines 0 comments Download
M client/dom/templates/dom/wrapping/wrapping_dom.js View 1 chunk +8 lines, -0 lines 0 comments Download
M client/tests/client/client.status View 1 1 chunk +1 line, -0 lines 0 comments Download
A client/tests/client/dom/DOMParserTest.dart View 1 1 chunk +14 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
nweiz
I'm waiting to submit this until I get all the tests running and passing.
8 years, 10 months ago (2012-02-04 01:26:22 UTC) #1
sra1
LGTM. Dartium folks: does anything need to be done about the native name in client/dom/src/native_FactoryProvidersImplementation.dart ...
8 years, 10 months ago (2012-02-04 01:39:10 UTC) #2
podivilov
On 2012/02/04 01:39:10, sra1 wrote: > LGTM. > > Dartium folks: does anything need to ...
8 years, 10 months ago (2012-02-06 12:18:10 UTC) #3
podivilov
https://chromiumcodereview.appspot.com/9310103/diff/1/client/dom/src/native_FactoryProvidersImplementation.dart File client/dom/src/native_FactoryProvidersImplementation.dart (right): https://chromiumcodereview.appspot.com/9310103/diff/1/client/dom/src/native_FactoryProvidersImplementation.dart#newcode7 client/dom/src/native_FactoryProvidersImplementation.dart:7: static DOMParser createDOMParser() native "DOMParser_constructor_Callback"; As discussed on dart ...
8 years, 10 months ago (2012-02-06 18:40:16 UTC) #4
antonm
8 years, 10 months ago (2012-02-06 18:52:23 UTC) #5
https://chromiumcodereview.appspot.com/9310103/diff/1/client/dom/src/native_F...
File client/dom/src/native_FactoryProvidersImplementation.dart (right):

https://chromiumcodereview.appspot.com/9310103/diff/1/client/dom/src/native_F...
client/dom/src/native_FactoryProvidersImplementation.dart:7: static DOMParser
createDOMParser() native "DOMParser_constructor_Callback";
On 2012/02/06 18:40:16, podivilov wrote:
> As discussed on dart dom meeting, you have 3 options here:
> 1) remove native binding and just throw unimplemented exception here
> 2) file a bug against Dartium to implement DOMParser_constructor_Callback
> 3) add a test that fails on Dartium and file a bug

I'd vote for 3rd---that would both increase coverage and make it harder to
forget about the problem.

Powered by Google App Engine
This is Rietveld 408576698