Bug 88405 - [Forms] We should share RenderStyle object for optgroup and option element
Summary: [Forms] We should share RenderStyle object for optgroup and option element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 03:09 PDT by yosin
Modified: 2014-08-14 11:56 PDT (History)
3 users (show)

See Also:


Attachments
patch (1.53 KB, patch)
2014-08-14 11:12 PDT, Antti Koivisto
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-06-06 03:09:05 PDT
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".
Comment 1 Antti Koivisto 2014-08-14 11:12:31 PDT
Created attachment 236603 [details]
patch
Comment 2 Antti Koivisto 2014-08-14 11:56:33 PDT
https://trac.webkit.org/r172597