New issue
Advanced search Search tips

Issue 829668 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

html/parser: "in frameset" insertion mode, <template> tag, implementation differs from spec

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

Issue description

The HTMLTreeBuilder.cpp implementation has a case for a <template> start tag, when in kInFramesetMode:
https://github.com/chromium/chromium/blob/fc665c309169a09fe5e1aee961a3c2fb4fb15563/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp#L1240

The HTML5 spec, section 12.2.6.4.20 The "in frameset" insertion mode,  does not have such a case:
https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inframeset

I'm not sure which is correct, between the implementation and the spec, but the two do differ. If it's the spec, how do I best file an upstream bug?

Thanks to Kunpei Sakai for the original report: https://go-review.googlesource.com/c/net/+/94838/9/html/parse.go#1906
 
Components: -Blink>HTML Blink>HTML>Parser
When I said "If it's the spec", I meant "If the spec is incorrect".

Separately, there's also a discrepancy in the 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://github.com/chromium/chromium/blob/fc665c309169a09fe5e1aee961a3c2fb4fb15563/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp#L1551

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.

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

Labels: Hotlist-Interop Hotlist-GoodFirstBug
Status: Available (was: Untriaged)
Confirmed that Firefox and Safari didn't recognize <template> in <frameset>.
The following HTML should not show "[object HTMLTemplateElement]".

<!DOCTYPE html>
<script>
window.onload = function() {
  alert(document.body.firstChild);
};
</script>
<frameset><template>
<div>abc</div>
</template></frameset>


Please file another bug for Comment 2.

Comment 2 spun out as issue #849934.

Comment 6 by ratsu...@gmail.com, Jun 13 2018

Hi all, I'd like to work on this issue

Comment 7 by tkent@chromium.org, Jun 14 2018

#6, Great!  Please go ahead.

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a464a0af1611a1c9d25253afc62238142eeb1a45

commit a464a0af1611a1c9d25253afc62238142eeb1a45
Author: Sunny <ratsunny@gmail.com>
Date: Fri Jun 22 01:33:54 2018

Remove non-standard template tag process code from html parser

According to HTML document parsing spec[1], there is no special
procedure for <template> tag when in "in frameset" insertion mode.
Besides, in "in template" insertion mode, the parser should not process
<frameset> tag by changing insertion mode to "in frameset" mode.

Around 18 failed tests should be passed after this change.

For standalone html5lib tests in LayoutTests/html5lib, template.dat
is updated to match the correct behavior.

[1] https://html.spec.whatwg.org/multipage/parsing.html

BUG= 829668 

Change-Id: Id25e6ab9ce47042a8e86a30aa2827f529f24f463
Reviewed-on: https://chromium-review.googlesource.com/1108191
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569497}
[delete] https://crrev.com/c642f7a962b87275d89cddbd47103cc6f8cef939/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-template-element/template-element/template-as-a-descendant-expected.txt
[delete] https://crrev.com/c642f7a962b87275d89cddbd47103cc6f8cef939/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-template-element/template-element/template-descendant-frameset-expected.txt
[delete] https://crrev.com/c642f7a962b87275d89cddbd47103cc6f8cef939/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_template_run_type=uri-expected.txt
[delete] https://crrev.com/c642f7a962b87275d89cddbd47103cc6f8cef939/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_template_run_type=write-expected.txt
[delete] https://crrev.com/c642f7a962b87275d89cddbd47103cc6f8cef939/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_template_run_type=write_single-expected.txt
[delete] https://crrev.com/c642f7a962b87275d89cddbd47103cc6f8cef939/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/template/additions-to-the-in-frameset-insertion-mode/end-tag-frameset-expected.txt
[modify] https://crrev.com/a464a0af1611a1c9d25253afc62238142eeb1a45/third_party/WebKit/LayoutTests/html5lib/resources/template.dat
[modify] https://crrev.com/a464a0af1611a1c9d25253afc62238142eeb1a45/third_party/blink/renderer/core/html/parser/html_tree_builder.cc

Comment 9 by tkent@chromium.org, Jun 22 2018

Labels: Target-69
Status: Fixed (was: Available)

Sign in to add a comment