Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

endIndex less than startIndex for implied tags #896

Closed
thewilkybarkid opened this issue Aug 5, 2021 · 4 comments · Fixed by #910
Closed

endIndex less than startIndex for implied tags #896

thewilkybarkid opened this issue Aug 5, 2021 · 4 comments · Fixed by #910

Comments

@thewilkybarkid
Copy link

thewilkybarkid commented Aug 5, 2021

If I have

const p = new Parser({ onclosetag(name) {
  console.log({
    name,
    startIndex: p.startIndex,
    endIndex: p.endIndex,
  })
}})

and I run

p.write("<p>Foo</p><hr>");

I get

{ name: 'p', startIndex: 6, endIndex: 9 }
{ name: 'hr', startIndex: 10, endIndex: 13 }

But if I don't include the optional end tag for the <p>:

p.write("<p>Foo<hr>");

I get

{ name: 'p', startIndex: 6, endIndex: 9 }
{ name: 'hr', startIndex: 10, endIndex: 9 }

The endIndex for the hr doesn't seem to have been set, so it's the same as for the <p>. This trips up PostHTML's sourceLocations option (posthtml/posthtml-parser#63), which finds an endIndex less than a startIndex. This option is now used by Parcel, leading to parcel-bundler/parcel#6672.

@thewilkybarkid
Copy link
Author

thewilkybarkid commented Aug 5, 2021

Just seen the AST explorer (useful!), so put my case from parcel-bundler/parcel#6672 (comment) at https://astexplorer.net/#/gist/a31bfb2e193e72e403256d885fd4b756/0153ebc4a547b3cbbf3003f033329740369e6aac (focusing on the <hr> shows the problem). (Edit: just spotted that's not the latest version; it does still happen on 6.1.0.)

@thewilkybarkid
Copy link
Author

thewilkybarkid commented Aug 7, 2021

After a bit of look, I think

this.onclosetag(el);
and
this.endIndex = this.tokenizer.getAbsoluteIndex();
are the problem lines: the inferred closing tag (i.e. the missing </p>) is given the indices of the new void element (<hr>), and as the endIndex is taken from the tokenizer directly (which has no knowledge of the inferred </p>) it's wrong.

I'm wondering though, what should the startIndex/endIndex be for the inferred onclosetag (if indeed that callback should be called at all)? Both 6 (so the beginning of the <hr>)? 6 and 9 (same as the <hr>? Maybe even both null?

@thewilkybarkid thewilkybarkid changed the title endIndex less than startIndex for a void element when not including optional closing tags endIndex less than startIndex for implied tags Aug 20, 2021
@thewilkybarkid
Copy link
Author

Seems like this happens on invalid HTML too: parcel-bundler/parcel#6672 (comment).

fb55 added a commit that referenced this issue Aug 20, 2021
Fixes #896

BREAKING: Some indices (primarily end indices) will have changed with this.
@fb55
Copy link
Owner

fb55 commented Aug 20, 2021

@thewilkybarkid Thank you so much for the report, and for digging into the details! I have opened #910 with a refactor of start/end indices, which fixes this and other issues with how indices work.

@fb55 fb55 closed this as completed in #910 Aug 20, 2021
fb55 added a commit that referenced this issue Aug 20, 2021
Fixes #896

BREAKING: Some indices (primarily end indices) will have changed with this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants