New issue
Advanced search Search tips

Issue 597781 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

HTMLTreeBuilder has many notImplemented()'s that are easily reached

Project Member Reported by esprehn@chromium.org, Mar 24 2016

Issue description

These seem to be very old, they came from when the parser was rewritten:
https://chromium.googlesource.com/chromium/blink/+/bda7109b69a27276a8458e4cc17682aed10eb1fc

We should probably implement whatever is missing or remove them and replace with TODO. You shouldn't hit a notImplemented() during normal operation.

When we switch to Chromium's NOTIMPLEMENTED() they start logging all the time in layout tests, instead of just when the WTF logging channel for notImplemented() is turned on which is bad.
See: https://codereview.chromium.org/1820083005
 
Cc: domenic@chromium.org
I think someone needs to sit down the spec and remove all of these notImplemented() calls.

ex.

https://chromium.googlesource.com/chromium/blink/+/73deabbe706a1671a636da0b0d08818be94137b0/Source/core/html/parser/HTMLTreeBuilder.cpp#2491

We should probably mark the script started?

ex.

https://chromium.googlesource.com/chromium/blink/+/73deabbe706a1671a636da0b0d08818be94137b0/Source/core/html/parser/HTMLTreeBuilder.cpp#2507

I don't get why this one is notImplemented(), it seems to do what I expected? If we're in the initial mode and we hit a start tag we invoke this method, which sets the default compat mode, which I think is expected. I don't know what about the algorithm was never implemented... looking at the spec it says:

""
12.2.5.4.1 The "initial" insertion mode

...

Anything else
If the document is not an iframe srcdoc document, then this is a parse error; set the Document to quirks mode.

In any case, switch the insertion mode to "before html", then reprocess the token.
""

Comment 2 by tkent@chromium.org, Mar 24 2016

Status: Available (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2016

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

commit 62d2d8f07c321e78398a12e1344773f8db169f13
Author: tkent <tkent@chromium.org>
Date: Mon Mar 28 23:28:29 2016

Replace notImplemented() in core/html/parser/ with DVLOG(1).

This is a part of the task removing notImplemented().
We can't replace notImplemented()s in core/html/parser/ with NOTIMPLEMENTED()
because they hit too frequently.

These logs are not visible by default even in Debug build.
They are visible by --vmodule flag. e.g.
  % run-webkit-tests --debug --additional-driver-flag=--vmodule=HTMLTreeBuilder=1

BUG= 596760 ,597781

Review URL: https://codereview.chromium.org/1836843002

Cr-Commit-Position: refs/heads/master@{#383605}

[modify] https://crrev.com/62d2d8f07c321e78398a12e1344773f8db169f13/third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp
[modify] https://crrev.com/62d2d8f07c321e78398a12e1344773f8db169f13/third_party/WebKit/Source/core/html/parser/HTMLScriptRunner.cpp
[modify] https://crrev.com/62d2d8f07c321e78398a12e1344773f8db169f13/third_party/WebKit/Source/core/html/parser/HTMLTokenizer.cpp
[modify] https://crrev.com/62d2d8f07c321e78398a12e1344773f8db169f13/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp

Project Member

Comment 4 by sheriffbot@chromium.org, Mar 29 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue.
The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by tkent@chromium.org, Mar 29 2017

Labels: -Type-Bug -Hotlist-Recharge-Cold Type-Task
Status: Available (was: Untriaged)
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 16 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by tkent@chromium.org, Apr 17 2018

Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)

Sign in to add a comment