New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 679069 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

.innerText throws on a blocklist of elements

Project Member Reported by tabatkins@chromium.org, Jan 6 2017

Issue description

Our code throws for a subset of elements when you try to use the .innerText setter (namely, void elements and a few others like <basefont>).  This appears to be a result of reverse-engineering the earlier IE implementation of the feature, as .innerText was undocumented at the time.

The current spec doesn't have a blocklist, Edge doesn't have the old IE behavior (and Firefox never did), and .textContent works on all of those elements, so it seems like we should just go ahead and remove the blocklist, and let the setter work on all elements.

Discussion: https://github.com/whatwg/html/issues/2222
 
Cc: domenic@chromium.org
Components: Blink>DOM
Labels: Hotlist-Interop OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Owner: dominicc@chromium.org
Status: Started (was: Untriaged)
outerText has a similar check in Chromium FWIW, but it seems the spec for outerText setting isn't there yet: https://github.com/whatwg/html/issues/668
NextAction: 2017-01-16
Status: Assigned (was: Started)
Let's wait for the upstream web platform tests to land.
Tests have landed:

- http://w3c-test.org/innerText/setter.html
- http://w3c-test.org/domparsing/createContextualFragment.html

(no tests for outerText though.)

Comment 6 by ratsu...@gmail.com, Feb 18 2017

Hi dominicc@

Seems there are three files [1] will use ieForbidsInsertHTML() to determine whether to place the end tag or not.

I'm not pretty sure how to change these files in Chromium code style, seems I have some choice:
1. Change HTMLElement::ieForbidsInsertHTML() to HTMLElement::cannotHaveEndTagWhenSerialising(), and keep the reference between these three files and the function.
2. Remove HTMLElement::ieForbidsInsertHTML(), for each place need to know whether it's a void element, use a big if statement to check it.
3. Like WebKit, remove HTMLElement::ieForbidsInsertHTML() entirely, use an array and a for loop in each file to check it.

So in Chromium project, which type of change you will choose?

Thanks in advance

[1]:
third_party/WebKit/Source/web/WebFrameSerializerImpl.cpp
third_party/WebKit/Source/core/editing/serializers/MarkupFormatter.cpp
third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.cpp

Comment 7 by tkent@chromium.org, Feb 20 2017

Cc: dominicc@chromium.org
Owner: ----
Status: Started (was: Assigned)
ratsunny@ is working on this.

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 22 2017

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

commit 7a581007693d6b482158d03cd6eac849da99bb76
Author: ratsunny <ratsunny@gmail.com>
Date: Wed Feb 22 09:50:38 2017

Remove blacklist on .innerHTML and .outerHTML

The blacklist was stored in HTMLElement::ieForbidsInsertHTML(),
used to disallow set inner/outerHTML on these elements and determine
whether the tag should be serailized with end tag or not.

The current spec doesn't have any blocklist on them, so the
blocklists on .innerHTML, .outerHTML and createContextualFragment()
are all removed in this change.

During the serialization, the spec [1] have a list of tag names
which should skip the end tag, the serializers now using it instead of
using the HTMLElement::ieForbidsInsertHTML().

This commit also fix a bug in serializeNodesWithNamespaces(), which
always append the end tag even it is serialized as html document and
the tag is in the list mentioned above.

Around 180 failed tests will be passed after this change.

[1] https://www.w3.org/TR/DOM-Parsing/

BUG= 679069 

Review-Url: https://codereview.chromium.org/2707973002
Cr-Commit-Position: refs/heads/master@{#451961}

[delete] https://crrev.com/c5608a34fd0112511e9165179a91cdd9277518b2/third_party/WebKit/LayoutTests/external/wpt/domparsing/createContextualFragment-expected.txt
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/LayoutTests/external/wpt/html/syntax/serializing-html-fragments/serializing-expected.txt
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/LayoutTests/external/wpt/innerText/setter-expected.txt
[delete] https://crrev.com/c5608a34fd0112511e9165179a91cdd9277518b2/third_party/WebKit/LayoutTests/fast/dom/HTMLElement/ie-forbids-insert-html-expected.txt
[delete] https://crrev.com/c5608a34fd0112511e9165179a91cdd9277518b2/third_party/WebKit/LayoutTests/fast/dom/HTMLElement/ie-forbids-insert-html.html
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.cpp
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/Source/core/editing/serializers/MarkupFormatter.cpp
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/Source/core/editing/serializers/Serialization.cpp
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/Source/core/html/HTMLElement.cpp
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/Source/core/html/HTMLElement.h
[modify] https://crrev.com/7a581007693d6b482158d03cd6eac849da99bb76/third_party/WebKit/Source/web/WebFrameSerializerImpl.cpp

Comment 9 by tkent@chromium.org, Feb 22 2017

Labels: M-58
NextAction: ----
Status: Fixed (was: Started)

Sign in to add a comment