[webkit-reviews] review granted: [Bug 235237] When inserting a selected <option> in a <select> element, its selected state should remain : [Attachment 449394] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 235237: When inserting a selected <option> in a <select> element, its
selected state should remain
https://bugs.webkit.org/show_bug.cgi?id=235237

Attachment 449394: Patch

https://bugs.webkit.org/attachment.cgi?id=449394&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 449394
  --> https://bugs.webkit.org/attachment.cgi?id=449394
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449394&action=review

> Source/WebCore/html/HTMLOptGroupElement.cpp:80
> +    for (auto* option = Traversal<HTMLOptionElement>::lastChild(*this);
option; option = Traversal<HTMLOptionElement>::previousSibling(*option)) {

Does this work?

    for (auto& option :
makeReversedRange(childrenOfType<HTMLOptionElement>(*this))) {

I think it should work and I like it better. But maybe makeReversedRange won’t
work until we add rbegin and rend to ElementChildRange? I’d want to do this
soon.

> Source/WebCore/html/HTMLSelectElement.cpp:374
>  void HTMLSelectElement::childrenChanged(const ChildChange& change)

Can we find a way to share more of the code between HTMLSelectElement and
HTMLOptGroupElement? Helper function or class?