New issue
Advanced search Search tips

Issue 756771 link

Starred by 1 user

Issue metadata

Status: Fixed
Merged: issue 611632
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 651762



Sign in to add a comment

Document named getter bug: |"foo" in document| is true though document.foo is undefined

Project Member Reported by tkent@chromium.org, Aug 18 2017

Issue description

Chrome Version: 62 ToT
OS: All but iOS

What steps will reproduce the problem?
(1) Open http://w3c-test.org/html/dom/documents/dom-tree-accessors/nameditem-06.html

What is the expected result?
No "Fail" tests

What happens instead?
Two "Fail" tests

Please use labels and text to provide additional information.
We shouldn't return element without |name| content attributes.
Edge and Firefox work correctly.

Spec: https://html.spec.whatwg.org/multipage/dom.html#dom-document-nameditem
Implementation is here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp?type=cs&sq=package:chromium&l=385

 

Comment 1 by tkent@chromium.org, Aug 18 2017

Description: Show this description

Comment 2 by phistuck@gmail.com, Aug 18 2017

Huh, it is weird, while it does put the ID as a named getter, it returns undefined.
So for data:text/html,<img id=test3> -
"test3" in document // true
document.test3 // undefined

Comment 3 by tkent@chromium.org, Aug 21 2017

Summary: Document named getter bug: |"foo" in document| is true though document.foo is undefined (was: Document named getter isn't interoperable)
> "test3" in document // true
> document.test3 // undefined

Yeah, it's very inconsistent behavior.

Comment 4 by peria@chromium.org, Aug 21 2017

Mergedinto: 611632
Status: Duplicate (was: Available)

Comment 5 by tkent@chromium.org, Aug 21 2017

Owner: tkent@chromium.org
Status: Started (was: Duplicate)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 21 2017

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

commit 92f4e9430d74094a7b6e943eba07f2f578650921
Author: Kent Tamura <tkent@chromium.org>
Date: Mon Aug 21 23:44:46 2017

Fix document name getter behavior for IMG with id attribute.

According to the specification [1], "id" attribute value of an IMG
element should be handled by Document named getter only if the IMG
element has non-empty "name" attribute.

Our implementation had two issues:
- V8 accessor was created for "id" attribute value regardless of "name"
  attribute value.
- The getter function handled name="" incorrectly.
 
Implementation:
We had Element::ShouldRegisterAsNamedItem() and
ShouldRegisterAsExtraNamedItem() to tell that an element should handle
"name" attribute and/or "id" attribute. They were not enough, and we
should add another class for IMG elements.

This CL introduce enum NamedNodeType, and Element::GetNamedNodeType() to
replace ShouldRegisterAsNamedItem() and ShouldRegisterAsExtraNamedItem().

As for the name="" issue,  DocumentNamedCollection.cpp is updated so
that it uses IsEmpty() instead of Element::HasName(), which is based on
IsNull().

[1] https://html.spec.whatwg.org/multipage/dom.html#dom-document-nameditem-filter

Bug:  756771 
Change-Id: I6b6931082580f79a7f73ea42c4e8128162608423
Reviewed-on: https://chromium-review.googlesource.com/622330
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496122}
[delete] https://crrev.com/87d7e6ddb22a1cf4916fcbb74b293ee5a8fa6227/third_party/WebKit/LayoutTests/external/wpt/html/dom/documents/dom-tree-accessors/nameditem-06-expected.txt
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/LayoutTests/fast/dom/HTMLDocument/document-special-properties-expected.txt
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/dom/Element.h
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/html/DocumentNameCollection.cpp
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/html/HTMLEmbedElement.h
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/html/HTMLFormElement.h
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/html/HTMLImageElement.h
[modify] https://crrev.com/92f4e9430d74094a7b6e943eba07f2f578650921/third_party/WebKit/Source/core/html/HTMLObjectElement.h

Comment 7 by tkent@chromium.org, Aug 22 2017

Labels: M-62
Status: Fixed (was: Started)

Sign in to add a comment