Bug 64504

Summary:

Focus and selection events are not fired when a <select>'s selection changes

Product:

WebKit

Reporter:

Jon Honeycutt <jhoneycutt>

Component:

Accessibility

Assignee:

Jon Honeycutt <jhoneycutt>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alice.barraclough, pnormand, rniwa, scheib

Priority:

Keywords:

InRadar, PlatformOnly

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

64681

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch v2	alice.barraclough: review+
Patch v3	webkit.review.bot: commit-queue-
Patch v4	alice.barraclough: review+

Jon Honeycutt

Reported 2011-07-13 19:29:29 PDT

WebKit should fire focus and selection events when the selected <option> in a <select> element changes. <rdar://problem/9319881>

Attachments
Patch (23.36 KB, patch) 2011-07-14 00:07 PDT, Jon Honeycutt	no flags	Details Formatted Diff Diff
Patch v2 (23.83 KB, patch) 2011-07-15 04:34 PDT, Jon Honeycutt	alice.barraclough: review+	Details Formatted Diff Diff
Patch v3 (24.37 KB, patch) 2011-07-18 05:24 PDT, Jon Honeycutt	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Patch v4 (24.38 KB, patch) 2011-07-18 05:47 PDT, Jon Honeycutt	alice.barraclough: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Jon Honeycutt

Comment 1 2011-07-14 00:07:30 PDT

Created attachment 100780 [details] Patch

Jon Honeycutt

Comment 2 2011-07-15 04:34:55 PDT

Created attachment 100957 [details] Patch v2 Try to fix Chromium build. Changed an AXObjectCache::getOrCreate() call to just get() in RenderMenuList::didUpdateActiveOption() - we don't need to create the object if it doesn't exist.

Alice Liu

Comment 3 2011-07-15 14:51:15 PDT

Comment on attachment 100957 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=100957&action=review r=me assuming the location of the early return in didSetSelectedIndex is either fine or fixed up. > Source/WebCore/rendering/RenderMenuList.cpp:353 > + It looks like didSetSelectedIndex used to set m_lastSelectedIndex even if accessibilityEnabled was false. Now you're not going to be setting m_lastActiveIndex if accessibility isn't enabled. I guess that's okay if m_lastActiveIndex is only ever read or written if accessibility is enabled, but i can't tell 100% from this patch. could you confirm this?

Jon Honeycutt

Comment 4 2011-07-15 16:27:21 PDT

(In reply to comment #3) > (From update of attachment 100957 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=100957&action=review > > r=me assuming the location of the early return in didSetSelectedIndex is either fine or fixed up. > > > Source/WebCore/rendering/RenderMenuList.cpp:353 > > + > > It looks like didSetSelectedIndex used to set m_lastSelectedIndex even if accessibilityEnabled was false. Now you're not going to be setting m_lastActiveIndex if accessibility isn't enabled. I guess that's okay if m_lastActiveIndex is only ever read or written if accessibility is enabled, but i can't tell 100% from this patch. could you confirm this? Good catch! m_lastSelectedIndex is only used by this accessibility code, so I'll leave it as-is. Thanks for the review!

Jon Honeycutt

Comment 5 2011-07-16 05:42:46 PDT

Landed in <http://trac.webkit.org/changeset/91132>. Build fix in <http://trac.webkit.org/changeset/91135>.

Ryosuke Niwa

Comment 6 2011-07-18 00:13:14 PDT

A bunch of tests seem to have start hitting assertions on Chromium bots after this patch was landed: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20chromium.org&tests=editing%2Fselection%2Fiframe.html%2Cdom%2Fhtml%2Flevel2%2Fhtml%2FHTMLOptGroupElement01.html%2Cdom%2Fhtml%2Flevel2%2Fhtml%2FHTMLOptGroupElement02.html%2Cediting%2Fpasteboard%2F4641033.html%2Cfast%2Fcss%2Fpseudo-required-optional-unapplied.html%2Cfast%2Fevents%2Ftabindex-focus-blur-all.html%2Cfast%2Fforms%2FValidityState-valueMissing-001.html%2Cfast%2Fforms%2Fform-collection-elements.html%2Cfast%2Fforms%2Foption-in-optgroup-removal.html%2Cfast%2Fforms%2Fselect-out-of-bounds-index.html%2Cfast%2Fforms%2Fselect-selected.html%2Cfast%2Fforms%2Fstuff-on-my-optgroup.html%2Cfast%2Finvalid%2Fresidual-style.html%2Cfast%2Fregex%2FtoString.html%2Chtml5lib%2Frunner.html%2Cediting%2Fselection%2Fmove-by-word-visually-others.html%2Cfast%2Fjs%2Ffunction-apply.html%2Cfast%2Fjs%2Freserved-words-as-property.html%2Cjquery%2Fcss.html%2Cjquery%2Fdata.html%2Cstorage%2Findexeddb%2Findex-cursor.html

Ryosuke Niwa

Comment 7 2011-07-18 00:19:00 PDT

Looking at http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug?numbuilds=50 and http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/24311 this patch also caused major failures on GTK :(

Philippe Normand

Comment 8 2011-07-18 00:22:26 PDT

GTK stack trace: #0 0x00002ae02f0b9016 in WebCore::RenderMenuList::didUpdateActiveOption (this=0xfb9eb28, optionIndex=0) at ../../Source/WebCore/rendering/RenderMenuList.cpp:362 362 ASSERT(toOptionElement(select->listItems()[optionIndex])); Thread 1 (Thread 15390): #0 0x00002ae02f0b9016 in WebCore::RenderMenuList::didUpdateActiveOption (this=0xfb9eb28, optionIndex=0) at ../../Source/WebCore/rendering/RenderMenuList.cpp:362 #1 0x00002ae02f0b801e in WebCore::RenderMenuList::setTextFromOption (this=0xfb9eb28, optionIndex=0) at ../../Source/WebCore/rendering/RenderMenuList.cpp:208 #2 0x00002ae02f0b7ead in WebCore::RenderMenuList::updateFromElement (this=0xfb9eb28) at ../../Source/WebCore/rendering/RenderMenuList.cpp:188 #3 0x00002ae02eade996 in WebCore::SelectElement::setSelectedIndex (data=..., element=0xfb2f2f0, optionIndex=0, deselect=true, fireOnChangeNow=false, userDrivenChange=false) at ../../Source/WebCore/dom/SelectElement.cpp:381 #4 0x00002ae02ec2f0ec in WebCore::HTMLSelectElement::setSelectedIndex (this=0xfb2f2f0, optionIndex=0, deselect=false) at ../../Source/WebCore/html/HTMLSelectElement.cpp:86 #5 0x00002ae02ec28fe3 in WebCore::HTMLOptionElement::insertedIntoTree (this=0xef04990, deep=false) at ../../Source/WebCore/html/HTMLOptionElement.cpp:251 #6 0x00002ae02ea13ec7 in WebCore::ContainerNode::insertedIntoDocument (this=0xef04990) at ../../Source/WebCore/dom/ContainerNode.cpp:784 #7 0x00002ae02ea744a3 in WebCore::Element::insertedIntoDocument (this=0xef04990) at ../../Source/WebCore/dom/Element.cpp:959 #8 0x00002ae02eae7825 in WebCore::StyledElement::insertedIntoDocument (this=0xef04990) at ../../Source/WebCore/dom/StyledElement.cpp:455 #9 0x00002ae02ebf5416 in WebCore::HTMLFormControlElement::insertedIntoDocument (this=0xef04990) at ../../Source/WebCore/html/HTMLFormControlElement.cpp:194 #10 0x00002ae02ea13a40 in WebCore::ContainerNode::parserAddChild (this=0xfb9f180, newChild=...) at ../../Source/WebCore/dom/ContainerNode.cpp:690 #11 0x00002ae02ec527fd in WebCore::HTMLConstructionSite::attach<WebCore::Element> (this=0xc7e9578, rawParent=0xfb9f180, prpChild=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:103 #12 0x00002ae02ec50738 in WebCore::HTMLConstructionSite::attachToCurrent (this=0xc7e9578, child=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:265 #13 0x00002ae02ec50be2 in WebCore::HTMLConstructionSite::insertHTMLElement (this=0xc7e9578, token=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:295 #14 0x00002ae02ec77c32 in WebCore::HTMLTreeBuilder::processStartTag (this=0xc7e9550, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:1429 #15 0x00002ae02ec72547 in WebCore::HTMLTreeBuilder::processToken (this=0xc7e9550, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:481 #16 0x00002ae02ec723f4 in WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken (this=0xc7e9550, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:462 #17 0x00002ae02ec72356 in WebCore::HTMLTreeBuilder::constructTreeFromToken (this=0xc7e9550, rawToken=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:452 #18 0x00002ae02ec54b14 in WebCore::HTMLDocumentParser::pumpTokenizer (this=0xef07310, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:276 #19 0x00002ae02ec54532 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0xef07310, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:175 #20 0x00002ae02ec55080 in WebCore::HTMLDocumentParser::append (this=0xef07310, source=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:367 #21 0x00002ae02ea18e8c in WebCore::DecodedDataDocumentParser::appendBytes (this=0xef07310, writer=0xf150550, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821) at ../../Source/WebCore/dom/DecodedDataDocumentParser.cpp:50 #22 0x00002ae02ed923aa in WebCore::DocumentWriter::addData (this=0xf150550, bytes=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821) at ../../Source/WebCore/loader/DocumentWriter.cpp:203 #23 0x00002ae02ed8632b in WebCore::DocumentLoader::commitData (this=0xf150450, bytes=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821) at ../../Source/WebCore/loader/DocumentLoader.cpp:321 #24 0x00002ae02e70c16d in WebKit::FrameLoaderClient::committedLoad (this=0x20ea130, loader=0xf150450, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821) at ../../Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:320 #25 0x00002ae02ed86203 in WebCore::DocumentLoader::commitLoad (this=0xf150450, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821) at ../../Source/WebCore/loader/DocumentLoader.cpp:307 #26 0x00002ae02ed863e6 in WebCore::DocumentLoader::receivedData (this=0xf150450, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821) at ../../Source/WebCore/loader/DocumentLoader.cpp:333 #27 0x00002ae02edd1267 in WebCore::MainResourceLoader::addData (this=0xef06600, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821, allAtOnce=false) at ../../Source/WebCore/loader/MainResourceLoader.cpp:168 #28 0x00002ae02eddeaaf in WebCore::ResourceLoader::didReceiveData (this=0xef06600, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821, encodedDataLength=3821, allAtOnce=false) at ../../Source/WebCore/loader/ResourceLoader.cpp:284 #29 0x00002ae02edd2696 in WebCore::MainResourceLoader::didReceiveData (this=0xef06600, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821, encodedDataLength=3821, allAtOnce=false) at ../../Source/WebCore/loader/MainResourceLoader.cpp:464 #30 0x00002ae02eddf3b4 in WebCore::ResourceLoader::didReceiveData (this=0xef06600, data=0xfb2c7c0 "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML//EN\">\n<html>\n<head>\n<title>required and basic valueMissing</title>\n<link rel=\"stylesheet\" href=\"../../fast/js/resources/js-test-style.css\">\n<script src=\"../../"..., length=3821, encodedDataLength=3821) at ../../Source/WebCore/loader/ResourceLoader.cpp:442 #31 0x00002ae02ef73f1b in WebCore::readCallback (source=0xb05f400, asyncResult=0xa9205e0, data=0x0) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:801 #32 0x00002ae032a5db59 in async_ready_callback_wrapper (source_object=0xb05f400, res=0xa9205e0, user_data=0x0) at /tmp/buildd/glib2.0-2.28.6/./gio/ginputstream.c:470 #33 0x00002ae032a6da68 in complete_in_idle_cb_for_thread (_data=0xfb553d0) at /tmp/buildd/glib2.0-2.28.6/./gio/gsimpleasyncresult.c:812 #34 0x00002ae0335db4a3 in g_main_dispatch (context=0x2073a40) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:2440 #35 g_main_context_dispatch (context=0x2073a40) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3013 #36 0x00002ae0335dbc80 in g_main_context_iterate (context=0x2073a40, block=1, dispatch=1, self=<value optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3091 #37 0x00002ae0335dc2f2 in g_main_loop_run (loop=0xfaeb880) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3299 #38 0x00002ae0315172b7 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #39 0x000000000042dc00 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:707 #40 0x000000000042d29d in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:499 #41 0x000000000042f558 in main (argc=2, argv=0x7ffff5b64fd8) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1187

Philippe Normand

Comment 9 2011-07-18 00:28:20 PDT

http://webkit-bots.igalia.com/amd64/svn_91171.core-when_1310958982-_-who_DumpRenderTree-_-why_11.trace.html is the link to the above stack-trace.

Ryosuke Niwa

Comment 10 2011-07-18 00:31:04 PDT

Per discussion on IRC, we're rolling this patch out: https://bugs.webkit.org/show_bug.cgi?id=64681 Sorry :(

Ryosuke Niwa

Comment 11 2011-07-18 00:37:40 PDT

fast/forms/option-in-optgroup-removal.html Chromium Windows ASSERTION FAILED: toOptionElement(select->listItems()[optionIndex]) Backtrace: WebCore::RenderMenuList::didUpdateActiveOption [0x02422B1C+220] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\rendering\rendermenulist.cpp:362) WebCore::RenderMenuList::setTextFromOption [0x02421D3D+285] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\rendering\rendermenulist.cpp:209) WebCore::RenderMenuList::updateFromElement [0x02421C09+169] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\rendering\rendermenulist.cpp:189) WebCore::updateFromElementCallback [0x016C0F97+215] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\htmlformcontrolelement.cpp:258) WebCore::ContainerNode::dispatchPostAttachCallbacks [0x017D9CCE+126] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\dom\containernode.cpp:746) WebCore::ContainerNode::resumePostAttachCallbacks [0x017D9ABE+46] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\dom\containernode.cpp:714) WebCore::Document::recalcStyle [0x01788758+936] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\dom\document.cpp:1563) WebCore::Document::updateStyleIfNeeded [0x01788A38+248] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\dom\document.cpp:1582) WebCore::Document::finishedParsing [0x01793752+322] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\dom\document.cpp:4193) WebCore::HTMLTreeBuilder::finished [0x017639B4+100] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\parser\htmltreebuilder.cpp:2825) WebCore::HTMLDocumentParser::end [0x01739503+131] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:379) WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd [0x017395F6+182] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:388) WebCore::HTMLDocumentParser::prepareToStopParsing [0x017382AC+188] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:152) WebCore::HTMLDocumentParser::attemptToEnd [0x01739669+57] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:399) WebCore::HTMLDocumentParser::finish [0x017397B3+51] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:427) WebCore::DocumentWriter::endIfNotLoadingMainResource [0x010DE429+201] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\loader\documentwriter.cpp:226) WebCore::DocumentWriter::end [0x010DE347+39] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\loader\documentwriter.cpp:210) WebCore::DocumentLoader::finishedLoading [0x011D2894+84] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\loader\documentloader.cpp:290) WebCore::FrameLoader::finishedLoading [0x0110FDC2+82] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:2045) WebCore::MainResourceLoader::didFinishLoading [0x01426890+304] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\loader\mainresourceloader.cpp:486) WebCore::ResourceLoader::didFinishLoading [0x0141F67B+91] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webcore\loader\resourceloader.cpp:449) WebCore::ResourceHandleInternal::didFinishLoading [0x022FB9F0+144] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\source\webkit\chromium\src\resourcehandle.cpp:190) webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest [0x00B503E5+533] (e:\b\build\slave\webkit_win__dbg__1_\build\src\webkit\glue\weburlloader_impl.cc:664) `anonymous namespace'::RequestProxy::NotifyCompletedRequest [0x00ACED69+57] (e:\b\build\slave\webkit_win__dbg__1_\build\src\webkit\tools\test_shell\simple_resource_loader_bridge.cc:274) [0x00AD6391+33] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\tuple.h:564) [0x00AD4AB5+53] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\task.h:338) `anonymous namespace'::TaskClosureAdapter::Run [0x01F07812+50] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:104) base::internal::Invoker1<0,base::internal::InvokerStorage1<void (__thiscall `anonymous namespace'::TaskClosureAdapter::*)(void),A0x17a47d7c::TaskClosureAdapter *>,void (__thiscall `anonymous namespace'::TaskClosureAdapter::*)(void)>::DoInvoke [0x01F1280D+45] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\bind_internal.h:595) base::Callback<void __cdecl(void)>::Run [0x00E0B47F+47] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\callback.h:265) MessageLoop::RunTask [0x01F08E75+293] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:486) MessageLoop::DeferOrRunPendingTask [0x01F08FC3+51] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:505) MessageLoop::DoWork [0x01F0999D+221] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:693) base::MessagePumpForUI::DoRunLoop [0x01F6BBA4+84] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_pump_win.cc:203) base::MessagePumpWin::RunWithDispatcher [0x01F6B472+130] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_pump_win.cc:51) base::MessagePumpWin::Run [0x01F6B6DC+28] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_pump_win.h:80) MessageLoop::RunInternal [0x01F08BC7+247] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:451) MessageLoop::RunHandler [0x01F0899E+46] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:425) MessageLoop::Run [0x01F082AA+58] (e:\b\build\slave\webkit_win__dbg__1_\build\src\base\message_loop.cc:349) webkit_support::RunMessageLoop [0x008E01BF+15] (e:\b\build\slave\webkit_win__dbg__1_\build\src\webkit\support\webkit_support.cc:375) TestShell::waitTestFinished [0x0042BCED+141] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\tools\dumprendertree\chromium\testshellwin.cpp:106) TestShell::runFileTest [0x0044646F+479] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\tools\dumprendertree\chromium\testshell.cpp:235) runTest [0x0041B12D+733] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\tools\dumprendertree\chromium\dumprendertree.cpp:125) main [0x0041A928+2232] (e:\b\build\slave\webkit_win__dbg__1_\build\src\third_party\webkit\tools\dumprendertree\chromium\dumprendertree.cpp:248) __tmainCRTStartup [0x00A74967+279] (f:\dd\vctools\crt_bld\self_x86\crt\src\crt0.c:266) mainCRTStartup [0x00A7483F+15] (f:\dd\vctools\crt_bld\self_x86\crt\src\crt0.c:182) RegisterWaitForInputIdle [0x7C817077+73]

Jon Honeycutt

Comment 12 2011-07-18 05:02:05 PDT

Reopening, since this was rolled out. Updated patch coming shortly.

Jon Honeycutt

Comment 13 2011-07-18 05:24:04 PDT

Created attachment 101150 [details] Patch v3 This should fix the logic error that was causing the assertion failures on the bots - the error being that an option index was used where a list index should've been used. Also note that an incorrect > was replaced with a >=. Here is the relevant diff: --- a/Source/WebCore/rendering/RenderMenuList.cpp +++ b/Source/WebCore/rendering/RenderMenuList.cpp @@ -356,10 +356,11 @@ void RenderMenuList::didUpdateActiveOption(int optionIndex) m_lastActiveIndex = optionIndex; SelectElement* select = toSelectElement(static_cast<Element*>(node())); - if (optionIndex < 0 || optionIndex > static_cast<int>(select->listItems().size())) + int listIndex = select->optionToListIndex(optionIndex); + if (listIndex < 0 || listIndex >= select->listItems().size()) return; - ASSERT(toOptionElement(select->listItems()[optionIndex])); + ASSERT(toOptionElement(select->listItems()[listIndex])); if (AccessibilityMenuList* menuList = static_cast<AccessibilityMenuList*>(document()->axObjectCache()->get(this))) menuList->didUpdateActiveOption(optionIndex);

WebKit Review Bot

Comment 14 2011-07-18 05:36:38 PDT

Comment on attachment 101150 [details] Patch v3 Attachment 101150 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9110391

Jon Honeycutt

Comment 15 2011-07-18 05:47:32 PDT

Created attachment 101152 [details] Patch v4 Fix a signed/unsigned comparison mismatch.

Jon Honeycutt

Comment 16 2011-07-18 16:17:20 PDT

Landed in r91219.

Note You need to log in before you can comment on or make changes to this bug.