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

Issue 745316 link

Starred by 3 users

Issue metadata

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

Blocking:
issue 651762



Sign in to add a comment

select.setAttribute('size', foo) should respect the specified string

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

Issue description

Chrome Version: 61 canary
OS: All but iOS

What steps will reproduce the problem?
(1) Open the following URL
data:text/html;charset=utf-8,<select size=foo><script>alert(document.querySelector('select').getAttribute('size'));</script>

What is the expected result?
A dialog shows "foo".

What happens instead?
A dialog shows "0".

Please use labels and text to provide additional information.
Edge, Firefox, and Safari work correctly.

HTMLSelectElement.cpp has the following code:

    // Set the attribute value to a number.
    // This is important since the style rules for this attribute can
    // determine the appearance property.
    unsigned size = params.new_value.GetString().ToUInt();
    AtomicString attr_size = AtomicString::Number(size);
    if (attr_size != params.new_value) {
      // FIXME: This is horribly factored.
      if (Attribute* size_attribute =
              EnsureUniqueElementData().Attributes().Find(sizeAttr))
        size_attribute->SetValue(attr_size);
    }

The comment is obsolete. We don't need such process now because we have :-internal-list-box pseudo selector.

WPT: http://w3c-test.org/html/dom/reflection-forms.html

 
Shall i start work on this bug?

Comment 2 by tkent@chromium.org, Jul 18 2017

Cc: sujith...@samsung.com
Status: Started (was: Available)
> Shall i start work on this bug?

Please go ahead.

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

Owner: tkent@chromium.org
sujiths.s@ looks not to work on this.  I have stated this.

tkent@,
already uploaded the patch, currently WIP
https://chromium-review.googlesource.com/c/582049

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

> already uploaded the patch, currently WIP
> https://chromium-review.googlesource.com/c/582049

Oh, it's too late, and your code change isn't correct.

Today I uploaded my CL, and it was already reviewed. I'm landing it.
https://chromium-review.googlesource.com/c/598763

Comment 6 Deleted

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 4 2017

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

commit 8de8f952cde5da47147368a7ddca24053056e3a0
Author: Kent Tamura <tkent@chromium.org>
Date: Fri Aug 04 03:53:49 2017

SELECT element: Do not normalize |size| content attribute value.

We normalized |size| content attribute value to a number in order to write the
UA stylesheet easier. We introduced :-internal-list-box a few years ago, and
this normalization isn't necessary now.

This CL does:

- Do not normalize |size| attribute value in HTMLSelectElement::
  ParseAttribute().
- Use the standard reflection code for |size| IDL attribute.

Bug:  745316 
Change-Id: Ifcd3b2b55762f49b72c49900ab89a4fe78fb9b78
Reviewed-on: https://chromium-review.googlesource.com/598763
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491933}
[modify] https://crrev.com/8de8f952cde5da47147368a7ddca24053056e3a0/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-forms-expected.txt
[modify] https://crrev.com/8de8f952cde5da47147368a7ddca24053056e3a0/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
[modify] https://crrev.com/8de8f952cde5da47147368a7ddca24053056e3a0/third_party/WebKit/Source/core/html/HTMLSelectElement.h
[modify] https://crrev.com/8de8f952cde5da47147368a7ddca24053056e3a0/third_party/WebKit/Source/core/html/HTMLSelectElement.idl

Comment 8 by tkent@chromium.org, Aug 4 2017

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

Sign in to add a comment