Changeset 124416 in webkit


Ignore:
Timestamp:
Aug 2, 2012 12:14:49 AM (12 years ago)
Author:
yosin@chromium.org
Message:

REGRESSION(r102741): [Forms] In selects, when disabled, browser skips first option if not in optgroup, then selects first option in optgroup
​https://bugs.webkit.org/show_bug.cgi?id=92833

Reviewed by Kent Tamura.

Source/WebCore:

This patch changes implementation of HTMLOptionElement::disabled() to
follow the "disabled" concept of option element in HTML5 specification[1],
the option element is disabled if option element has "disabled"
attribute or parent optgroup element has "disabled" attribute. Before
this patch, HTMLOptionElement::disabled() checks presenting "disabled"
attribute in option element itself and any parent element.

Before this patch, HTMLSelectElement::recalcListItems() didn't considers
non-disabled option as default selected option if select element is
disabled because HTMLOptionElement::disabled() returned true if select
element is disabled.

After this patch, HTMLOptionElement::disabled() is independent from
select element. HTMLSelectElement::recalcListItems() considers
non-disabled option as default selected option.

[1] ​http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#concept-option-disabled

Tests: fast/forms/basic-selects.html: Fixed expectation to right thing.

  • css/html.css:

(select[disabled]>option): Added to render option elements in disabled
select element to disabled color as before this patch.

  • html/HTMLOptionElement.cpp:

(WebCore::HTMLOptionElement::disabled): Changed to check parent element
is optgroup.

  • html/HTMLSelectElement.cpp:

(WebCore::HTMLSelectElement::listBoxDefaultEventHandler): On mouse up
and down, don't update selection if select element is disabled.

  • rendering/RenderListBox.cpp:

(WebCore::RenderListBox::paintItemForeground): Added checking select
element is disabled. Before this patch, it was done by HTMLOptionElement::disabled().

LayoutTests:

This patch updates test expectation of fast/forms/basic-selects.html
for Chromium-Linux and disables it for Chromium-Mac and Chromium-Win.

Note: We need to rebaseline for all ports expect for Chromium-Linux.

  • platform/chromium-linux/fast/forms/basic-selects-expected.png: Changed for default selected option for disabled select element, "foo" to "bar" of second select element of "Line-height should be ignored" line.
  • platform/chromium-linux/fast/forms/basic-selects-expected.txt:
  • platform/chromium/TestExpectations: Disabled fast/forms/basic-selects.html for Chromium-Mac and Chromium-Win.
Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r124415 r124416  
     12012-08-02  Yoshifumi Inoue  <yosin@chromium.org>
     2
     3        REGRESSION(r102741): [Forms] In selects, when disabled, browser skips first option if not in optgroup, then selects first option in optgroup
     4        https://bugs.webkit.org/show_bug.cgi?id=92833
     5
     6        Reviewed by Kent Tamura.
     7
     8        This patch updates test expectation of fast/forms/basic-selects.html
     9        for Chromium-Linux and disables it for Chromium-Mac and Chromium-Win.
     10
     11        Note: We need to rebaseline for all ports expect for Chromium-Linux.
     12
     13        * platform/chromium-linux/fast/forms/basic-selects-expected.png: Changed for default selected option for disabled select element, "foo" to "bar" of second select element of "Line-height should be ignored" line.
     14        * platform/chromium-linux/fast/forms/basic-selects-expected.txt:
     15        * platform/chromium/TestExpectations: Disabled fast/forms/basic-selects.html for Chromium-Mac and Chromium-Win.
     16
    1172012-08-02  Alexander Pavlov  <apavlov@chromium.org>
    218

  • trunk/LayoutTests/platform/chromium-linux/fast/forms/basic-selects-expected.txt

    r117821 r124416  
    4747        RenderMenuList {SELECT} at (237,83) size 43x20 [color=#808080] [bgcolor=#DDDDDD] [border: (1px solid #808080)]
    4848          RenderBlock (anonymous) at (1,1) size 41x18
    49             RenderText at (4,1) size 17x16
    50               text run at (4,1) width 17: "foo"
     49            RenderText at (4,1) size 18x16
     50              text run at (4,1) width 18: "bar"
    5151        RenderText {#text} at (282,83) size 8x19
    5252          text run at (282,83) width 8: "b"

  • trunk/LayoutTests/platform/chromium/TestExpectations

    r124415 r124416  
    34863486BUGCR139493 DEBUG : media/track/track-cues-sorted-before-dispatch.html = PASS CRASH
    34873487
     3488// Need rebaseline
     3489BUGWK92833 MAC WIN : fast/forms/basic-selects.html = IMAGE+TEXT PASS
     3490
    34883491// Supposedly started failing between CR r140760 and CR r141216.  The failures
    34893492// look like they involve antialiasing; the fact that the test clearly expects

  • trunk/Source/WebCore/ChangeLog

    r124413 r124416  
     12012-08-02  Yoshifumi Inoue  <yosin@chromium.org>
     2
     3        REGRESSION(r102741): [Forms] In selects, when disabled, browser skips first option if not in optgroup, then selects first option in optgroup
     4        https://bugs.webkit.org/show_bug.cgi?id=92833
     5
     6        Reviewed by Kent Tamura.
     7
     8        This patch changes implementation of HTMLOptionElement::disabled() to
     9        follow the "disabled" concept of option element in HTML5 specification[1],
     10        the option element is disabled if option element has "disabled"
     11        attribute or parent optgroup element has "disabled" attribute. Before
     12        this patch, HTMLOptionElement::disabled() checks presenting "disabled"
     13        attribute in option element itself and any parent element.
     14
     15        Before this patch, HTMLSelectElement::recalcListItems() didn't considers
     16        non-disabled option as default selected option if select element is
     17        disabled because HTMLOptionElement::disabled() returned true if select
     18        element is disabled.
     19
     20        After this patch, HTMLOptionElement::disabled() is independent from
     21        select element. HTMLSelectElement::recalcListItems() considers
     22        non-disabled option as default selected option.
     23
     24        [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#concept-option-disabled
     25
     26        Tests: fast/forms/basic-selects.html: Fixed expectation to right thing.
     27
     28        * css/html.css:
     29        (select[disabled]>option): Added to render option elements in disabled
     30        select element to disabled color as before this patch.
     31        * html/HTMLOptionElement.cpp:
     32        (WebCore::HTMLOptionElement::disabled): Changed to check parent element
     33        is optgroup.
     34        * html/HTMLSelectElement.cpp:
     35        (WebCore::HTMLSelectElement::listBoxDefaultEventHandler): On mouse up
     36        and down, don't update selection if select element is disabled.
     37        * rendering/RenderListBox.cpp:
     38        (WebCore::RenderListBox::paintItemForeground): Added checking select
     39        element is disabled. Before this patch, it was done by HTMLOptionElement::disabled().
     40
    1412012-08-01  Sheriff Bot  <webkit.review.bot@gmail.com>
    242

  • trunk/Source/WebCore/css/html.css

    r124407 r124416  
    692692input[type="button"]:disabled, input[type="submit"]:disabled, input[type="reset"]:disabled,
    693693input[type="file"]:disabled::-webkit-file-upload-button, button:disabled,
    694 select:disabled, keygen:disabled, optgroup:disabled, option:disabled {
     694select:disabled, keygen:disabled, optgroup:disabled, option:disabled,
     695select[disabled]>option {
    695696    color: GrayText
    696697}

  • trunk/Source/WebCore/html/HTMLOptionElement.cpp

    r123283 r124416  
    330330bool HTMLOptionElement::disabled() const
    331331{
    332     return ownElementDisabled() || (parentNode() && parentNode()->isHTMLElement() && static_cast<HTMLElement*>(parentNode())->disabled());
     332    if (ownElementDisabled())
     333        return true;
     334
     335    if (!parentNode() || !parentNode()->isHTMLElement())
     336        return false;
     337
     338    HTMLElement* parentElement = static_cast<HTMLElement*>(parentNode());
     339    return parentElement->hasTagName(optgroupTag) && parentElement->disabled();
    333340}
    334341

  • trunk/Source/WebCore/html/HTMLSelectElement.cpp

    r122115 r124416  
    12931293        int listIndex = toRenderListBox(renderer())->listIndexAtOffset(toSize(localOffset));
    12941294        if (listIndex >= 0) {
     1295            if (!disabled()) {
    12951296#if PLATFORM(MAC) || (PLATFORM(CHROMIUM) && OS(DARWIN))
    1296             updateSelectedState(listIndex, mouseEvent->metaKey(), mouseEvent->shiftKey());
     1297                updateSelectedState(listIndex, mouseEvent->metaKey(), mouseEvent->shiftKey());
    12971298#else
    1298             updateSelectedState(listIndex, mouseEvent->ctrlKey(), mouseEvent->shiftKey());
     1299                updateSelectedState(listIndex, mouseEvent->ctrlKey(), mouseEvent->shiftKey());
    12991300#endif
     1301            }
    13001302            if (Frame* frame = document()->frame())
    13011303                frame->eventHandler()->setMouseDownMayStartAutoscroll();
    … …  
    13111313        int listIndex = toRenderListBox(renderer())->listIndexAtOffset(toSize(localOffset));
    13121314        if (listIndex >= 0) {
    1313             if (m_multiple) {
    1314                 setActiveSelectionEndIndex(listIndex);
    1315                 updateListBoxSelection(false);
    1316             } else {
    1317                 setActiveSelectionAnchorIndex(listIndex);
    1318                 setActiveSelectionEndIndex(listIndex);
    1319                 updateListBoxSelection(true);
     1315            if (!disabled()) {
     1316                if (m_multiple) {
     1317                    setActiveSelectionEndIndex(listIndex);
     1318                    updateListBoxSelection(false);
     1319                } else {
     1320                    setActiveSelectionAnchorIndex(listIndex);
     1321                    setActiveSelectionEndIndex(listIndex);
     1322                    updateListBoxSelection(true);
     1323                }
    13201324            }
    13211325            event->setDefaultHandled();

  • trunk/Source/WebCore/rendering/RenderListBox.cpp

    r124389 r124416  
    379379    FontCachePurgePreventer fontCachePurgePreventer;
    380380
    381     const Vector<HTMLElement*>& listItems = toHTMLSelectElement(node())->listItems();
     381    HTMLSelectElement* selectElement = toHTMLSelectElement(node());
     382
     383    const Vector<HTMLElement*>& listItems = selectElement->listItems();
    382384    HTMLElement* element = listItems[listIndex];
    383385
    … …  
    402404            textColor = theme()->activeListBoxSelectionForegroundColor();
    403405        // Honor the foreground color for disabled items
    404         else if (!element->disabled())
     406        else if (!element->disabled() && !selectElement->disabled())
    405407            textColor = theme()->inactiveListBoxSelectionForegroundColor();
    406408    }
Note: See TracChangeset for help on using the changeset viewer.