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

Added new tests for foster parenting in template, after after frameset a... #26

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

inscriber
Copy link

...nd parsing fragment in html context.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/431

This is an external review system which you may optionally use for the code review of your pull request.

| content
| <template>
| content
| <div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble following this one. The div element is inserted targeting the table element with foster parenting enabled. For the div to end up before the table, we need the following:

  1. If last table has a parent element, then let adjusted insertion location be inside last table's parent element, immediately before last table, and abort these sub steps.

    must be true.

  2. If the adjusted insertion location is inside a template element, let it instead be inside the template element's template contents, after its last child (if any).

    must be false.

So from 1 the last table has a parent element, the template, and the adjusted insertion location is inside that parent element.
But from 2 the adjusted insertion location is not inside a template element. This contradicts 1 above.

By my reading, the error is in 2, and the div should instead go after the table.

@inscriber inscriber closed this Nov 18, 2013
@inscriber inscriber reopened this Nov 18, 2013
@inscriber
Copy link
Author

That's a good catch. The new foster parenting algorithm behaves differently from the stand alone spec for templates and I'm not sure if this outcome is intentional.

In Blink and Gecko, the adjusted insertion location in inside the template before the table, but as you mentioned, the new spec indicates that it should be after. This also means the previous test in template.dat is now incorrect as well. I'll raise a spec bug about this if you haven't already.

@nolanw
Copy link
Contributor

nolanw commented Nov 18, 2013

I think the test with the "foo" character tokens is fine. It gets to

If there is a last template and either there is no last table, or there is one, but last template is lower (more recently added) than last table in the stack of open elements, then: let adjusted insertion location be inside last template's template contents, after its last child (if any), and abort these sub steps.

which is where the characters were inserted in the test.

I have not yet filed a bug against the spec, so by all means go ahead!

@inscriber
Copy link
Author

I was referring to the test that we already have in template.dat

<template><template><table>Foo

I'm going to email Hixie to see if the behavior change was intentional. If so, I'll update both tests.

@nolanw
Copy link
Contributor

nolanw commented Nov 19, 2013

Oh, yep, that test is also affected. Good call.

@gsnedders
Copy link
Member

@revite: Any news on this? (Is the email public, or is there a bug on the spec? If so, a link would be nice!)

@inscriber
Copy link
Author

@gsnedders: Sorry for the late reply. Hixie got back to me and he has updated the spec so that the submitted tests are correct and the spec behavior matches the current browser behaviour.

In particular, sub-step 5 was reworded to "If last table has a parent node, then let adjusted insertion location be inside last table's parent node, immediately before last table, and abort these substeps." The key difference is changing the word "element" to "node".

The parser appends children of the template element into the template content document fragment instead of the element. The parent of the table in this case is a node, not a template element, thus it does not get readjusted by step 3.

http://www.whatwg.org/specs/web-apps/current-work/multipage/tree-construction.html#appropriate-place-for-inserting-a-node

@inscriber
Copy link
Author

This pull request now include another commit with tests covering changes relating to form pointer and template contents. http://html5.org/tools/web-apps-tracker?from=8330&to=8331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants