New issue
Advanced search Search tips

Issue 849934 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

html/parser: reset the insertion mode, implementation differs from spec

Project Member Reported by nigeltao@chromium.org, Jun 6 2018

Issue description

There's a discrepancy in the HTML5 parser algorithm's ResetInsertionModeAppropriately implementation, regarding the "last" variable.

The spec, 12.2.4.1 The insertion mode, says that handling <td> or <th> is conditional on the "last" variable:
https://html.spec.whatwg.org/multipage/parsing.html#the-insertion-mode

The implementation has no such conditionality:
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/parser/html_tree_builder.cc?l=1551&rcl=06f66cde750e1f388e19a0390214bb988eed1768

Similarly, the spec and implementation differ on whether handling <head> depends on the state of "last". The C++ code for handling <head> also conditionally modifies the fragment_context_, which doesn't appear to have analogous words in the spec.

 
If somebody's looking at HTMLTreeBuilder::ResetInsertionModeAppropriately, the existing "if (item->HasTagName(templateTag))" fragment is also in a different order than the spec. The C++ code places it early, before looking for <select>. The spec places it later, between processing <table> and <head>.

Comment 2 by tkent@chromium.org, Jun 6 2018

Cc: tkent@chromium.org
Thank you for filing this.
Can your provide a reproducing HTML like https://bugs.chromium.org/p/chromium/issues/detail?id=829668#c3 ?

Comment 3 by tkent@chromium.org, Jun 13 2018

Labels: -Type-Bug Hotlist-CodeHealth Type-Task
Status: Available (was: Untriaged)
In my understanding, this issue doesn't affect any behavior.

If 'last' is true, Blink code applies "in cell" insertion mode though the specification say it should be "in body" insertion mode.
<td> or <th> can be 'last' only for td.innerHTML setter and th.innerHTML setter. In such cases, "in cell" insertion mode is identical to "in body" insertion mode because 'stack of open elements' doesn't contain table-related elements.

This is just a source-code readability issue.

Cc: ratsu...@gmail.com
CC'ing Sunny because they recently landed https://chromium-review.googlesource.com/c/chromium/src/+/1108191 "Remove non-standard template tag process code from html parser" that fixed https://bugs.chromium.org/p/chromium/issues/detail?id=829668 ""in frameset" insertion mode, <template> tag, implementation differs from spec".

Sunny, please don't feel obliged to own this issue (849934, spun out of 829668), but if it's really easy for you to fix this one too (if it's not already fixed), that would be wonderful!

Sign in to add a comment