- Revision
- 124416
- Author
- [email protected]
- Date
- 2012-08-02 00:14:49 -0700 (Thu, 02 Aug 2012)
Log 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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (124415 => 124416)
--- trunk/LayoutTests/ChangeLog 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/LayoutTests/ChangeLog 2012-08-02 07:14:49 UTC (rev 124416)
@@ -1,3 +1,19 @@
+2012-08-02 Yoshifumi Inoue <[email protected]>
+
+ 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.
+
+ 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.
+
2012-08-02 Alexander Pavlov <[email protected]>
[Chromium] Unreviewed, mark fast/js/dfg-osr-entry-hoisted-clobbered-structure-check.html as DEBUG SLOW = PASS.
Modified: trunk/LayoutTests/platform/chromium/TestExpectations (124415 => 124416)
--- trunk/LayoutTests/platform/chromium/TestExpectations 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/LayoutTests/platform/chromium/TestExpectations 2012-08-02 07:14:49 UTC (rev 124416)
@@ -3485,6 +3485,9 @@
BUGCR139493 DEBUG : media/track/track-cues-sorted-before-dispatch.html = PASS CRASH
+// Need rebaseline
+BUGWK92833 MAC WIN : fast/forms/basic-selects.html = IMAGE+TEXT PASS
+
// Supposedly started failing between CR r140760 and CR r141216. The failures
// look like they involve antialiasing; the fact that the test clearly expects
// these pixels not to be antialiased means these shouldn't just be rebaselined
Modified: trunk/LayoutTests/platform/chromium-linux/fast/forms/basic-selects-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-linux/fast/forms/basic-selects-expected.txt (124415 => 124416)
--- trunk/LayoutTests/platform/chromium-linux/fast/forms/basic-selects-expected.txt 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/LayoutTests/platform/chromium-linux/fast/forms/basic-selects-expected.txt 2012-08-02 07:14:49 UTC (rev 124416)
@@ -46,8 +46,8 @@
text run at (228,83) width 7: "a"
RenderMenuList {SELECT} at (237,83) size 43x20 [color=#808080] [bgcolor=#DDDDDD] [border: (1px solid #808080)]
RenderBlock (anonymous) at (1,1) size 41x18
- RenderText at (4,1) size 17x16
- text run at (4,1) width 17: "foo"
+ RenderText at (4,1) size 18x16
+ text run at (4,1) width 18: "bar"
RenderText {#text} at (282,83) size 8x19
text run at (282,83) width 8: "b"
RenderBR {BR} at (290,83) size 0x19
Modified: trunk/Source/WebCore/ChangeLog (124415 => 124416)
--- trunk/Source/WebCore/ChangeLog 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/Source/WebCore/ChangeLog 2012-08-02 07:14:49 UTC (rev 124416)
@@ -1,3 +1,43 @@
+2012-08-02 Yoshifumi Inoue <[email protected]>
+
+ 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.
+
+ 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().
+
2012-08-01 Sheriff Bot <[email protected]>
Unreviewed, rolling out r124406.
Modified: trunk/Source/WebCore/css/html.css (124415 => 124416)
--- trunk/Source/WebCore/css/html.css 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/Source/WebCore/css/html.css 2012-08-02 07:14:49 UTC (rev 124416)
@@ -691,7 +691,8 @@
input[type="button"]:disabled, input[type="submit"]:disabled, input[type="reset"]:disabled,
input[type="file"]:disabled::-webkit-file-upload-button, button:disabled,
-select:disabled, keygen:disabled, optgroup:disabled, option:disabled {
+select:disabled, keygen:disabled, optgroup:disabled, option:disabled,
+select[disabled]>option {
color: GrayText
}
Modified: trunk/Source/WebCore/html/HTMLOptionElement.cpp (124415 => 124416)
--- trunk/Source/WebCore/html/HTMLOptionElement.cpp 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/Source/WebCore/html/HTMLOptionElement.cpp 2012-08-02 07:14:49 UTC (rev 124416)
@@ -329,7 +329,14 @@
bool HTMLOptionElement::disabled() const
{
- return ownElementDisabled() || (parentNode() && parentNode()->isHTMLElement() && static_cast<HTMLElement*>(parentNode())->disabled());
+ if (ownElementDisabled())
+ return true;
+
+ if (!parentNode() || !parentNode()->isHTMLElement())
+ return false;
+
+ HTMLElement* parentElement = static_cast<HTMLElement*>(parentNode());
+ return parentElement->hasTagName(optgroupTag) && parentElement->disabled();
}
Node::InsertionNotificationRequest HTMLOptionElement::insertedInto(ContainerNode* insertionPoint)
Modified: trunk/Source/WebCore/html/HTMLSelectElement.cpp (124415 => 124416)
--- trunk/Source/WebCore/html/HTMLSelectElement.cpp 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/Source/WebCore/html/HTMLSelectElement.cpp 2012-08-02 07:14:49 UTC (rev 124416)
@@ -1292,11 +1292,13 @@
IntPoint localOffset = roundedIntPoint(renderer()->absoluteToLocal(mouseEvent->absoluteLocation(), false, true));
int listIndex = toRenderListBox(renderer())->listIndexAtOffset(toSize(localOffset));
if (listIndex >= 0) {
+ if (!disabled()) {
#if PLATFORM(MAC) || (PLATFORM(CHROMIUM) && OS(DARWIN))
- updateSelectedState(listIndex, mouseEvent->metaKey(), mouseEvent->shiftKey());
+ updateSelectedState(listIndex, mouseEvent->metaKey(), mouseEvent->shiftKey());
#else
- updateSelectedState(listIndex, mouseEvent->ctrlKey(), mouseEvent->shiftKey());
+ updateSelectedState(listIndex, mouseEvent->ctrlKey(), mouseEvent->shiftKey());
#endif
+ }
if (Frame* frame = document()->frame())
frame->eventHandler()->setMouseDownMayStartAutoscroll();
@@ -1310,13 +1312,15 @@
IntPoint localOffset = roundedIntPoint(renderer()->absoluteToLocal(mouseEvent->absoluteLocation(), false, true));
int listIndex = toRenderListBox(renderer())->listIndexAtOffset(toSize(localOffset));
if (listIndex >= 0) {
- if (m_multiple) {
- setActiveSelectionEndIndex(listIndex);
- updateListBoxSelection(false);
- } else {
- setActiveSelectionAnchorIndex(listIndex);
- setActiveSelectionEndIndex(listIndex);
- updateListBoxSelection(true);
+ if (!disabled()) {
+ if (m_multiple) {
+ setActiveSelectionEndIndex(listIndex);
+ updateListBoxSelection(false);
+ } else {
+ setActiveSelectionAnchorIndex(listIndex);
+ setActiveSelectionEndIndex(listIndex);
+ updateListBoxSelection(true);
+ }
}
event->setDefaultHandled();
}
Modified: trunk/Source/WebCore/rendering/RenderListBox.cpp (124415 => 124416)
--- trunk/Source/WebCore/rendering/RenderListBox.cpp 2012-08-02 07:09:18 UTC (rev 124415)
+++ trunk/Source/WebCore/rendering/RenderListBox.cpp 2012-08-02 07:14:49 UTC (rev 124416)
@@ -378,7 +378,9 @@
{
FontCachePurgePreventer fontCachePurgePreventer;
- const Vector<HTMLElement*>& listItems = toHTMLSelectElement(node())->listItems();
+ HTMLSelectElement* selectElement = toHTMLSelectElement(node());
+
+ const Vector<HTMLElement*>& listItems = selectElement->listItems();
HTMLElement* element = listItems[listIndex];
RenderStyle* itemStyle = element->renderStyle();
@@ -401,7 +403,7 @@
if (frame()->selection()->isFocusedAndActive() && document()->focusedNode() == node())
textColor = theme()->activeListBoxSelectionForegroundColor();
// Honor the foreground color for disabled items
- else if (!element->disabled())
+ else if (!element->disabled() && !selectElement->disabled())
textColor = theme()->inactiveListBoxSelectionForegroundColor();
}
