88405 2012-06-06 03:09:05 -0700 [Forms] We should share RenderStyle object for optgroup and option element 2014-08-14 11:56:33 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 yosin webkit-unassigned esprehn kling koivisto oldest_to_newest 642319 0 yosin 2012-06-06 03:09:05 -0700 When we tried to share RenderStyle object for optgroup and option elements, see patch for bug 88059. We failed on following two tests: 1 fast/css/text-transform-select.html 2 fast/forms/select/menulist-disabled-option.html == fast/css/text-transform-select.html == RenderText object doesn't apply text transformation when calling setStyle. This is occurred in RenderMenuList::setText if (m_buttonText && !m_buttonText->isBR()) m_buttonText->setText(s.impl(), true); else { if (m_buttonText) m_buttonText->destroy(); m_buttonText = new (renderArena()) RenderText(document(), s.impl()); m_buttonText->setStyle(style()); When we don't share RenderStyle object, setText is called for each option element and m_buttonText->setText is called for same option more than once == RenderText::setText is called more than once. === Proposed Fix === It seems we create RenderText once then reuse it. == fast/forms/select/menulist-disabled-option.html == The problem was we called HTMLSelectElement::listItems() during DOM construction. The first disabled option was marked selected when it was added. === Proposed Fix === It seems it is better to store selected state into HTMLSelectElement rather than HTMLOptionElement object. Because "selected" attribute of the "option" element doesn't denote selected state, e.g. <select><option selected value="1">One</option><option selected value="2">Two</option> In this HTML, selected option is the first one even if second "option" has attribute "selected". 1029146 1 236603 koivisto 2014-08-14 11:12:31 -0700 Created attachment 236603 patch 1029164 2 koivisto 2014-08-14 11:56:33 -0700 https://trac.webkit.org/r172597 236603 2014-08-14 11:12:31 -0700 2014-08-14 11:16:09 -0700 patch option-style-share.patch text/plain 1569 koivisto SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3MjU5NCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE0LTA4LTE0ICBBbnR0aSBL b2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KKworICAgICAgICBbRm9ybXNdIFdlIHNob3VsZCBz aGFyZSBSZW5kZXJTdHlsZSBvYmplY3QgZm9yIG9wdGdyb3VwIGFuZCBvcHRpb24gZWxlbWVudAor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODg0MDUKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZSBjYW4gcmVt b3ZlIHRoaXMgc3BlY2lhbCBjYXNlIG5vdy4KKworICAgICAgICAqIGNzcy9TdHlsZVJlc29sdmVy LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlN0eWxlUmVzb2x2ZXI6OmNhblNoYXJlU3R5bGVXaXRo RWxlbWVudCk6CisKIDIwMTQtMDgtMTQgIEFudHRpIEtvaXZpc3RvICA8YW50dGlAYXBwbGUuY29t PgogCiAgICAgICAgIFJlbW92ZSBub25SZW5kZXJlclN0eWxlCkluZGV4OiBTb3VyY2UvV2ViQ29y ZS9jc3MvU3R5bGVSZXNvbHZlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvY3Nz L1N0eWxlUmVzb2x2ZXIuY3BwCShyZXZpc2lvbiAxNzI1ODApCisrKyBTb3VyY2UvV2ViQ29yZS9j c3MvU3R5bGVSZXNvbHZlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTYyOCwxMiArNjI4LDYgQEAg Ym9vbCBTdHlsZVJlc29sdmVyOjpjYW5TaGFyZVN0eWxlV2l0aEVsZQogICAgIGlmIChlbGVtZW50 LT5oYXNJRCgpICYmIG1fcnVsZVNldHMuZmVhdHVyZXMoKS5pZHNJblJ1bGVzLmNvbnRhaW5zKGVs ZW1lbnQtPmlkRm9yU3R5bGVSZXNvbHV0aW9uKCkuaW1wbCgpKSkKICAgICAgICAgcmV0dXJuIGZh bHNlOwogCi0gICAgLy8gRklYTUU6IFdlIHNob3VsZCBzaGFyZSBzdHlsZSBmb3Igb3B0aW9uIGFu ZCBvcHRncm91cCB3aGVuZXZlciBwb3NzaWJsZS4KLSAgICAvLyBCZWZvcmUgZG9pbmcgc28sIHdl IG5lZWQgdG8gcmVzb2x2ZSBpc3N1ZXMgaW4gSFRNTFNlbGVjdEVsZW1lbnQ6OnJlY2FsY0xpc3RJ dGVtcwotICAgIC8vIGFuZCBSZW5kZXJNZW51TGlzdDo6c2V0VGV4dC4gU2VlIGFsc28gaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg4NDA1Ci0gICAgaWYgKGlzSFRNTE9w dGlvbkVsZW1lbnQoZWxlbWVudCkgfHwgaXNIVE1MT3B0R3JvdXBFbGVtZW50KGVsZW1lbnQpKQot ICAgICAgICByZXR1cm4gZmFsc2U7Ci0KICAgICBib29sIGlzQ29udHJvbCA9IGVsZW1lbnQtPmlz Rm9ybUNvbnRyb2xFbGVtZW50KCk7CiAKICAgICBpZiAoaXNDb250cm9sICE9IHN0YXRlLmVsZW1l bnQoKS0+aXNGb3JtQ29udHJvbEVsZW1lbnQoKSkK