| Summary: | [Chromium] fix a layout test crash except for mac (LayoutTest/fast/forms/select-script-onchange.html) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Takashi Toyoshima <toyoshim> | ||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED DUPLICATE | ||||||||
| Severity: | Normal | CC: | eric, tkent, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Linux | ||||||||
| Attachments: |
|
||||||||
Created attachment 94954 [details]
Patch
Attachment 94954 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/ChangeLog:6: Line contains tab character. [whitespace/tab] [5]
LayoutTests/ChangeLog:10: Line contains tab character. [whitespace/tab] [5]
Total errors found: 2 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
This change use navigator.userAgent to switch codes which depends on platform. Another idea is preparing two different tests for each platform and just skip a test for another. Actually, I'm not familier with platform dependent tests. So I could not decide which is better approach. Created attachment 94956 [details]
Fix style bugs
Comment on attachment 94956 [details] Fix style bugs (In reply to comment #0) > It cause calling WebViewHost::createPopupMenu() on SPACE_OR_RETURN_POP_MENU platform, > and it's not implemented by DumpRenderTree for chromium > (Tools/DumpRenderTree/chromium/WebViewHost.cpp:241). Right. So we should implement popup menu in DRT/Chromium. > Originally "\r" must be intended to call htmlForm->submitImplicitly() on ARROW_KEYS_POP_MENU platform. No. Select elements in the test don't have their parent forms. So submitImplicitly() is not called. Hi, Kent-san. Thank you for review. (In reply to comment #5) > Right. So we should implement popup menu in DRT/Chromium. OK. I understand DRT/Chromium needs pseudo popup implementation. But, originally this test doesn't require popup menu. So I think it's independent issue. This test works on Mac as... 1. popup.focus() Set active focus to second select form. 2. eventSender.keyDown("t", null); Select the second item 'two' by initial word 't'. 3. eventSender.keyDown("\r", null); Fix the selection. And on Linux and Windows as... 1. popup.focus() Set active focus to second select form. 2. eventSender.keyDown("t", null); Select the second item 'two' by initial word 't'. 3. eventSender.keyDown("\r", null); Show pulldown menu for select items. The process 3 cause quite different action for Linux and Windows. Because pop-up keystrokes configuration is different as I described. Popup menu is not intended. That's why I just skip the "\r" event in this change. (In reply to comment #6) > But, originally this test doesn't require popup menu. It's right. But showing a pop-menu should be harmless. Actually this test doesn't fail on Qt and Apple Windows. So we don't need to remove the \r. > It's right. But showing a pop-menu should be harmless. Actually this test doesn't fail on Qt and Apple Windows. So we don't need to remove the \r.
OK.
I see the background.
I withdraw this patch.
What's the status of this old patch? I think the problem was resolved by another patch. https://bugs.webkit.org/show_bug.cgi?id=63857 *** This bug has been marked as a duplicate of bug 63857 *** |
LayoutTest/fast/forms/select-script-onchange.html crashes on chromium-linux and chromium-win. Pop-up keystrokes depends on platform and classed into two groups. As WebCore/dom/SelectElement.cpp said like: // Configure platform-specific behavior when focused pop-up receives arrow/space/return keystroke. // (PLATFORM(MAC) and PLATFORM(GTK) are always false in Chromium, hence the extra tests.) #if PLATFORM(MAC) || (PLATFORM(CHROMIUM) && OS(DARWIN)) #define ARROW_KEYS_POP_MENU 1 #define SPACE_OR_RETURN_POP_MENU 0 #elif PLATFORM(GTK) || (PLATFORM(CHROMIUM) && OS(UNIX)) #define ARROW_KEYS_POP_MENU 0 #define SPACE_OR_RETURN_POP_MENU 1 #else #define ARROW_KEYS_POP_MENU 0 #define SPACE_OR_RETURN_POP_MENU 0 #endif This test sends "t\r" keyDown event to change the selected item. It cause calling WebViewHost::createPopupMenu() on SPACE_OR_RETURN_POP_MENU platform, and it's not implemented by DumpRenderTree for chromium (Tools/DumpRenderTree/chromium/WebViewHost.cpp:241). Originally "\r" must be intended to call htmlForm->submitImplicitly() on ARROW_KEYS_POP_MENU platform. But this function isn't called on SPACE_OR_RETURN_POP_MENU platform. So, I just skip it on SPACE_OR_RETURN_POP_MENU platform. Crash dump is following [11297:11297:4333840847280:ERROR:process_util_posix.cc(108)] Received signal 11 base::debug::StackTrace::StackTrace() [0x91b81a] base::(anonymous namespace)::StackDumpSignalHandler() [0x8e42cb] 0x7f8ebbed3af0 WebKit::WebPopupMenuImpl::Init() [0x4fbb82] WebKit::ChromeClientImpl::popupOpened() [0x4be183] WebCore::PopupContainer::showPopup() [0xd6cf6d] WebCore::PopupContainer::showInRect() [0xd6d98c] WebCore::PopupMenuChromium::show() [0xd71993] WebCore::RenderMenuList::showPopup() [0x159fbb2] WebCore::SelectElement::menuListDefaultEventHandler() [0x10a0a3b] WebCore::SelectElement::defaultEventHandler() [0x10a1a9c] WebCore::HTMLSelectElement::defaultEventHandler() [0x675866] WebCore::EventDispatcher::dispatchEvent() [0x1046f51] WebCore::EventDispatchMediator::dispatchEvent() [0x104572c] WebCore::KeyboardEventDispatchMediator::dispatchEvent() [0x1054729] WebCore::EventDispatcher::dispatchEvent() [0x10458dd] WebCore::Node::dispatchKeyEvent() [0x106819f] WebCore::EventHandler::keyEvent() [0x126ba76] WebKit::WebViewImpl::charEvent() [0x4a8a5f] WebKit::WebViewImpl::handleInputEvent() [0x4aa330] EventSender::keyDown() [0x43083d] (snip)