Bug 79764

Duplicate of bug 77502? What does profiler say?

Yes, root cause of this bug and bug 77502 is same. I changed bug summary to clarify what I will do == make HTMLOptionElement not to inherit FormAssociateElement. I'll do same thing other non-form associate elements, such as legend, optgroup, etc.

(In reply to comment #0) > Imported from http://crbug.com/90094 This is unfriendly bug description. You should have written what was the bug instead of the URL.

(In reply to comment #1) > Duplicate of bug 77502? No, see comment #2 >What does profiler say? I measure running time of my desktop http://jsfiddle.net/pcc7Z/ Insert into div takes 319ms Insert into form takes 2113ms

Created attachment 132553 [details] Patch 1

Comment on attachment 132553 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=132553&action=review > Source/WebCore/css/CSSStyleSelector.cpp:1349 > + if (element->hasTagName(optionTag)) > + return false; > + This is wrong. It means we don't share a style for <option> elements in any cases. We should check: if both of m_element and element are optionTag, if enabled state is different, false if checked state is different, false otherwise, fall-through (means we can share the style) > LayoutTests/fast/forms/resources/select-live-pseudo-selectors.js:55 > + if (typeof(el) == 'string') { > + el = document.getElementById(el); inconsistent indentation.

Created attachment 132558 [details] Patch 2

Comment on attachment 132558 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=132558&action=review > Source/WebCore/css/CSSStyleSelector.cpp:1355 > + if (thisOptionElement->disabled() != otherOptionElement->disabled()) > + return false; nit: We should use isEnabledFormControl() for consistency with SelectorChecker.cpp.

Created attachment 132942 [details] Patch 3

Comment on attachment 132942 [details] Patch 3 ok

Comment on attachment 132942 [details] Patch 3 Clearing flags on attachment: 132942 Committed r111497: <http://trac.webkit.org/changeset/111497>

All reviewed patches have been landed. Closing bug.

Reopen, because it broke 2 tests on all platform: - fast/css/text-transform-select.html - fast/forms/select/menulist-disabled-option.html

(In reply to comment #13) > Reopen, because it broke 2 tests on all platform: > - fast/css/text-transform-select.html > - fast/forms/select/menulist-disabled-option.html See https://bugs.webkit.org/show_bug.cgi?id=81746 for details.

(In reply to comment #6) > (From update of attachment 132553 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132553&action=review > > > Source/WebCore/css/CSSStyleSelector.cpp:1349 > > + if (element->hasTagName(optionTag)) > > + return false; > > + > > This is wrong. It means we don't share a style for <option> elements in any cases. > We should check: > if both of m_element and element are optionTag, > if enabled state is different, false > if checked state is different, false > otherwise, fall-through (means we can share the style) > It seems this changes current behavior. Current implementation doesn't share style for HTMLOptionElement, HTMLOptionElement isn't HTMLInputElement. bool CSSStyleSelector::canShareStyleWithControl(StyledElement* element) const { HTMLInputElement* thisInputElement = element->toInputElement(); HTMLInputElement* otherInputElement = m_element->toInputElement(); if (!thisInputElement || !otherInputElement) return false;

(In reply to comment #15) > (In reply to comment #6) > > (From update of attachment 132553 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=132553&action=review > > > > > Source/WebCore/css/CSSStyleSelector.cpp:1349 > > > + if (element->hasTagName(optionTag)) > > > + return false; > > > + > > > > This is wrong. It means we don't share a style for <option> elements in any cases. > > We should check: > > if both of m_element and element are optionTag, > > if enabled state is different, false > > if checked state is different, false > > otherwise, fall-through (means we can share the style) > > > > It seems this changes current behavior. > Current implementation doesn't share style for HTMLOptionElement, HTMLOptionElement isn't HTMLInputElement. ok, let's revert this part. Did you reproduced the test failures with r111497, and confirmed they were fixed by reverting this part?

> > > > Source/WebCore/css/CSSStyleSelector.cpp:1349 > > > > + if (element->hasTagName(optionTag)) > > > > + return false; > > > > + > > > > > > This is wrong. It means we don't share a style for <option> elements in any cases. > > > We should check: > > > if both of m_element and element are optionTag, > > > if enabled state is different, false > > > if checked state is different, false > > > otherwise, fall-through (means we can share the style) > > > > > > > It seems this changes current behavior. > > Current implementation doesn't share style for HTMLOptionElement, HTMLOptionElement isn't HTMLInputElement. > > ok, let's revert this part. > Did you reproduced the test failures with r111497, and confirmed they were fixed by reverting this part? Confirmed on Mac port. I'm not sure why Chromium-Linux port passed test but not others. (><)

Created attachment 133196 [details] Patch 4

Comment on attachment 133196 [details] Patch 4 Clearing flags on attachment: 133196 Committed r111659: <http://trac.webkit.org/changeset/111659>

All reviewed patches have been landed. Closing bug.