RESOLVED FIXED 208102
Fix HTMLDataListElement.options to include even options that are not suggestions
https://bugs.webkit.org/show_bug.cgi?id=208102
Summary Fix HTMLDataListElement.options to include even options that are not suggestions
Darin Adler
Reported 2020-02-22 23:26:11 PST
Fix HTMLDataListElement.options to include even options that are not suggestions
Attachments
Patch (17.93 KB, patch)
2020-02-22 23:31 PST, Darin Adler
no flags
Darin Adler
Comment 1 2020-02-22 23:31:49 PST
Darin Adler
Comment 2 2020-02-23 07:16:13 PST
Radar WebKit Bug Importer
Comment 3 2020-02-23 07:17:15 PST
Tim Horton
Comment 4 2020-03-10 19:35:47 PDT
Comment on attachment 391481 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391481&action=review > Source/WebCore/html/ColorInputType.cpp:291 > - suggestions.reserveInitialCapacity(length); > - for (unsigned i = 0; i != length; ++i) { > - auto value = downcast<HTMLOptionElement>(*options->item(i)).value(); > - if (isValidSimpleColor(value)) > - suggestions.uncheckedAppend(Color(value)); > + for (auto& option : dataList->suggestions()) { > + if (auto color = parseSimpleColorValue(option.value())) > + suggestions.uncheckedAppend(*color); Concerning that EWS didn't catch the loss of reserveInitialCapacity here
Darin Adler
Comment 5 2020-03-10 19:37:55 PDT
Comment on attachment 391481 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391481&action=review >> Source/WebCore/html/ColorInputType.cpp:291 >> + suggestions.uncheckedAppend(*color); > > Concerning that EWS didn't catch the loss of reserveInitialCapacity here Oops. Obviously this needs to be plain old append, not uncheckedAppend! I can’t believe I missed that.
Darin Adler
Comment 6 2020-03-10 19:38:37 PDT
Comment on attachment 391481 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391481&action=review >>> Source/WebCore/html/ColorInputType.cpp:291 >>> + suggestions.uncheckedAppend(*color); >> >> Concerning that EWS didn't catch the loss of reserveInitialCapacity here > > Oops. Obviously this needs to be plain old append, not uncheckedAppend! I can’t believe I missed that. Are you going to fix that, Tim, or would you like me to?
Tim Horton
Comment 7 2020-03-10 20:54:27 PDT
I think Megan is fixing it! (she found the problem, I just did the spelunking to find out how it got this way).
Note You need to log in before you can comment on or make changes to this bug.