Internal type error with duplicate attributes and generateSpans: true

[refi64]

Repro:

import 'package:html/parser.dart' as html;

void main(List<String> args) {
  var dom = html.parse('<div duplicate duplicate>', generateSpans: true);
  print(dom.querySelector('div').attributeSpans);
}

This results in:

Unhandled exception:
type 'ParseErrorToken' is not a subtype of type 'StartTagToken' in type cast where
  ParseErrorToken is from package:html/src/token.dart
  StartTagToken is from package:html/src/token.dart

#0      Object._as (dart:core-patch/object_patch.dart:73)
#1      Node._ensureAttributeSpans (package:html/dom.dart:286:35)
#2      Node.attributeSpans (package:html/dom.dart:173:5)
#3      main (file:///khome/ryan/langtest/dart/html_bug/bin/html_bug.dart:5:34)
#4      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:263)
#5      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:151)

[kevmoo]

@kirbyfan64 – what would you expect? Just ignore the duplicate attributes?

The code change isn't hard – just not sure it's the right thing to do.

@jmesserly ?

diff --git a/lib/dom.dart b/lib/dom.dart
index fb0d458..53c8851 100644
--- a/lib/dom.dart
+++ b/lib/dom.dart
@@ -284,6 +284,10 @@ abstract class Node {
         generateSpans: true, attributeSpans: true);

     tokenizer.moveNext();
+
+    while (tokenizer.current is ParseErrorToken) {
+      tokenizer.moveNext();
+    }
     var token = tokenizer.current as StartTagToken;

     if (token.attributeSpans == null) return; // no attributes

[refi64]

I mean, I don't mind if it throws an error...just an intentional one, not an "internal" type error like this.

[jmesserly]

Good catch on that bug! And that fix LGTM.

[kevmoo]

I'll fix to throw – at least it will throw prettier

…

On Mon, Jan 8, 2018 at 11:49 AM, Jenny Messerly ***@***.***> wrote: Good catch on that bug! And that fix LGTM. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#56 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCiqRM_4yYVwh3qS3WjULPFY_e0jm5ks5tInFWgaJpZM4Q1fpH> .

[jmesserly]

I'll fix to throw – at least it will throw prettier

yeah either way. The *Span APIs are provided by this package (rather than being part of the real DOM spec), so we can decide the behavior. Typically HTML does try pretty hard to recover a DOM tree of some sort, even in the face of parse errors. So I think your first patch above is my slight preference.

[kevmoo]

So duplicates should be ignored? Hrm...I I think I'll improve the existing error and then we can discuss. Don't want to get into the business of changing behavior...

…

On Mon, Jan 8, 2018 at 11:58 AM, Jenny Messerly ***@***.***> wrote: I'll fix to throw – at least it will throw prettier yeah either way. The *Span APIs are provided by this package (rather than being part of the real DOM spec), so we can decide the behavior. Typically HTML does try pretty hard to recover a DOM tree of some sort, even in the face of parse errors. So I think your first patch above is my slight preference. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#56 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCivuWSyK_M_SHcBrrnHcubSl6lr7wks5tInNOgaJpZM4Q1fpH> .

[jmesserly]

Hrm...I I think I'll improve the existing error and then we can discuss.

sounds good to me!