Wrong filter for datalist.options |
||
Issue descriptionChrome Version: (copy from chrome://version) OS: (e.g. Win7, OSX 10.9.5, etc...) What steps will reproduce the problem? (1) Open http://w3c-test.org/html/semantics/forms/the-datalist-element/datalistoptions.html What is the expected result? No FAIL result What happens instead? There is one FAIL result. Please use labels and text to provide additional information. https://html.spec.whatwg.org/multipage/forms.html#dom-datalist-options > The options IDL attribute must return an HTMLCollection rooted at the datalist node, whose filter matches option elements. HTMLDataListOptionsCollection.h: const HTMLOptionElement& option = toHTMLOptionElement(element); if (!option.isDisabledFormControl() && !option.value().isEmpty()) return true; The |if| is unnecessary. Note that removing it might affect <datalist> popup UI. Edge: PASS Firefox: expected 5 but got 4.
,
Mar 13 2017
It's case by case. As for fast/forms/datalist/datalist.html, we may remove it because the WPT test covers the correct behavior. We should keep the current behavior for existing Blink-internal callsites of HTMLDataListElement::options() [1] except V8HTMLDataListElement.cpp. That is to say, we should add disabled and empty value checks to each of the callsites. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/HTMLDataListElement.h?type=cs&sq=package:chromium&l=47
,
Mar 13 2017
Thanks, I will let you know if I have any questions. I'm working on this in https://codereview.chromium.org/2746823002/
,
Mar 13 2017
I think the following changes would fix the issue: https://codereview.chromium.org/2746823002/
,
Mar 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d08576a42a3c298ce3b8a39f92f58440446e303d commit d08576a42a3c298ce3b8a39f92f58440446e303d Author: martin <martin@martinrogalla.com> Date: Tue Mar 14 02:35:51 2017 Correct elementMatches filter for datalist.options The filter should match all option elements [1], even if they are empty or disabled. This patch fixes the filter and ensures that the tests in [2] pass. All callers have been updated to filter on empty and disabled values, except for the call from V8HTMLDataListElement. The `fast/forms/datalist/datalist.html` test was removed as the correct behavior is checked by `wpt/html/semantics/forms/the-datalist-element/datalistoptions.html`. [1] - https://html.spec.whatwg.org/multipage/forms.html#dom-datalist-options [2] - http://w3c-test.org/html/semantics/forms/the-datalist-element/datalistoptions.html BUG= 687073 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2746823002 Cr-Commit-Position: refs/heads/master@{#456586} [delete] https://crrev.com/f1c3dbc05ae362562665108771589d20b1d499ac/third_party/WebKit/LayoutTests/external/wpt/html/semantics/forms/the-datalist-element/datalistoptions-expected.txt [delete] https://crrev.com/f1c3dbc05ae362562665108771589d20b1d499ac/third_party/WebKit/LayoutTests/fast/forms/datalist/datalist-expected.txt [delete] https://crrev.com/f1c3dbc05ae362562665108771589d20b1d499ac/third_party/WebKit/LayoutTests/fast/forms/datalist/datalist.html [modify] https://crrev.com/d08576a42a3c298ce3b8a39f92f58440446e303d/third_party/WebKit/Source/core/html/HTMLDataListOptionsCollection.h [modify] https://crrev.com/d08576a42a3c298ce3b8a39f92f58440446e303d/third_party/WebKit/Source/core/html/HTMLInputElement.cpp [modify] https://crrev.com/d08576a42a3c298ce3b8a39f92f58440446e303d/third_party/WebKit/Source/core/html/forms/ColorInputType.cpp [modify] https://crrev.com/d08576a42a3c298ce3b8a39f92f58440446e303d/third_party/WebKit/Source/core/html/forms/RangeInputType.cpp [modify] https://crrev.com/d08576a42a3c298ce3b8a39f92f58440446e303d/third_party/WebKit/Source/core/paint/ThemePainter.cpp
,
Mar 14 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by martin@martinrogalla.com
, Mar 13 2017