Skip to content

Test additions to expose/clarify perviously uncovered distinctions between "self closing tags" and void elements#1023

Closed
vassudanagunta wants to merge 1 commit intofb55:masterfrom
vassudanagunta:master
Closed

Test additions to expose/clarify perviously uncovered distinctions between "self closing tags" and void elements#1023
vassudanagunta wants to merge 1 commit intofb55:masterfrom
vassudanagunta:master

Conversation

@vassudanagunta
Copy link
Copy Markdown
Contributor

This PR is first a forum for clarification, using test code for demonstration. Whether or not all or part of the test code changes should be accepted, modified or refactored depends on the outcome of this clarification.

The issue covered are:

  • distinction between "self closing tags" and "void elements"

    The parser makes this distinction implicitly. I was able to piece this together from comments made in previous Issues/Pull Requests (where such a distinction is asserted) many years ago, from the code, and from the existence of the recognizeSelfClosing option. But there is no documentation on this distinction, even though void elements employ self closing tags. Either these terms should be synonymous or void elements is a subset of self closing tags.

    It's unclear whether this distinction makes sense. Or whether it makes sense that in HTML mode the self-closing /> tag pattern is ignored and an onclosetag event is generated later in the event stream no where near the actual self-closing tag.

  • The meaning and intent of the recognizeSelfClosing option.

    This option was introduced in Fix existing and add new options #74 but without any tests nor any documentation.

    To confirm there is no test coverage:

    1. Temporarily add the following line to Parser.ts's constructor:

      this.options.recognizeSelfClosing = !this.options.recognizeSelfClosing
      

      This will invert this option for every test, which will cause any test covering this option to fail.

    2. Run the tests. No tests will fail.

    Some documentation has been added since, but the distinction between "self closing tags" and "void elements" is still unclear, as is the real meaning of recognizeSelfClosing.

  • Parser.ts has special logic for <br> vs other void elements.

    But it's hard to figure out what the effective difference is.

  • The lack of tests covering/documenting the above.

The following test changes were made. All the tests PASS, so actual behavior is demonstrated. Questions raised are flagged with 🌶.

  • 07-self-closing.json has extended input:

    • Both "self-closing tags" and "void elements" to expose behavioral differences. Compare the different results for <hr /> (void element) and <break /> (self-closing tag).

      🌶 clearly there is a difference. Is what the test shows intentional?

    • The addition of <br /> to expose any different treatment or lack thereof.

      🌶 no apparent difference. (see also test 35 below)

    • A void element with attributes to add coverage of that scenario.

      Seems a-ok.

  • 07b-self-closing.json was added. It has the exact same input as 07-self-closing.json, but the Parser'srecognizeSelfClosing is set to true.

    🌶 Diff 07 and 07a to see the differences.

    Is the point of the recognizeSelfClosing option to allow self-closing tags that are not standard HTML "void elements" to be supported? i.e. does it exist to address Self closing tags with periods in their name are parsed nested instead of as siblings #1015 but without having to enable all the other parts of xmlMode?

  • 35-non-br-void-close-tag.json has extended input: <br> was added to show whether <br> in fact has different treatment as the name of the test implies.

    🌶 The test exposed no differences. The events for <br> are identical to <input>. Then what exactly does this special logic do?

Happy to make any changes or refactors to these tests.

…havior.

- distinction between "self closing tags" and "void elements"
- The meaning and intent of the `recognizeSelfClosing` option.
- `Parser.ts` special logic for `<br>` vs other void elements.
@vassudanagunta
Copy link
Copy Markdown
Contributor Author

I will be resubmitting this with some mods, and from another branch than vassudanagunta:master as I have another thing to PR.

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 this pull request may close these issues.

1 participant