Bug 58924

The Windows failure is because Flash is missing, based on the output text. Other tests used Flash, so I assumed that would be fine (I can't use the test plugin because it doesn't consume clicks). Do we just need to skip the test there?

Oh, I see. On Win it failed, rather than timed out. Jessie, should we check in the failing result or put the test on the Skipped list?

If it is skipped on Mac (in the mac/Skipped file), then it will automatically be skipped on Win because win/ inherits from mac/.

(In reply to comment #3) > If it is skipped on Mac (in the mac/Skipped file), then it will automatically be skipped on Win because win/ inherits from mac/. Oh, I didn't know that. Is it possible to run the test on win but not mac? Should I remove the entry from win/Skipped? http://trac.webkit.org/changeset/84287

This is failing on the chromium bots as well ... http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=%20plugins%2Fmouse-click-iframe-to-plugin.html%20&group=%40ToT%20-%20chromium.org

This was my first experience writing this kind of test; I thought I understood what I was doing based on the other tests I looked at, and the fact that it worked in local testing, but clearly I'm missing something important. It would be really helpful if someone with a better understanding of these tests could look at it and let me know if I made some n00b mistake.

I'll rework the test and attempt to first get it working first on chromium mac.

Created attachment 92390 [details] Patch Sent this patch to the chromium layout try bots, awaiting results.

http://build.chromium.org/p/tryserver.chromium/builders/mac_layout/builds/573 2011-05-05 01:56:54,649 13273 worker.py:218 DEBUG worker/1 plugins/mouse-click-iframe-to-plugin.html passed 2011-05-05 01:56:54,649 13273 worker.py:218 DEBUG worker/1 plugins/mouse-click-iframe-to-plugin.html passed 2011-05-05 01:56:54,649 13260 printing.py:538 INFO plugins/mouse-click-iframe-to-plugin.html -> unexpected pass

Comment on attachment 92390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92390&action=review Do you want this patched to be reviewed? Stuart, does this test rewrite seem fine to you? > LayoutTests/plugins/mouse-click-iframe-to-plugin.html:9 > + if (window.layoutTestController) > + layoutTestController.dumpAsText(); We normally just left align the <script> contents. > LayoutTests/plugins/mouse-click-iframe-to-plugin.html:17 > + var s = getComputedStyle(textarea, null).getPropertyValue("outline-style"); > + var w = getComputedStyle(textarea, null).getPropertyValue("outline-width"); Using getComputedStyle seems fine, but why didn't frameDocument.activeElement work?

(In reply to comment #10) > Stuart, does this test rewrite seem fine to you? Without understanding what was wrong with the test as I wrote it, it's hard for me to evaluate a rewrite.

(In reply to comment #10) > Do you want this patched to be reviewed? Nope, just wanted to first see if I could make chrome-mac co-operate. This needs a better test to address the issues reported on bug 58419 - focus the textarea, then click on the plugin, then type in the textarea. > > LayoutTests/plugins/mouse-click-iframe-to-plugin.html:9 > > + if (window.layoutTestController) > > + layoutTestController.dumpAsText(); > > We normally just left align the <script> contents. I'll fix that in the proceedings. > > LayoutTests/plugins/mouse-click-iframe-to-plugin.html:17 > > + var s = getComputedStyle(textarea, null).getPropertyValue("outline-style"); > > + var w = getComputedStyle(textarea, null).getPropertyValue("outline-width"); > > Using getComputedStyle seems fine, but why didn't frameDocument.activeElement work? http://build.webkit.org/old-results/Windows%207%20Release%20(Tests)/r84283%20(11851)/plugins/mouse-click-iframe-to-plugin-actual.txt MISSING PLUGIN BUTTON PRESSED ... There's no Flash installed on the test system, so the test was clicked on the missing plugin and testing the missing plugin code paths, not the code added on r84270. In Safari win/mac you can load a plugin of type="application/foo-bar", and then click the missing plugin. Safari does not move the focus to a missing plugin in my testing of it. Screen shot of me clicking the plugin OBJECT tag in Safari 5.0.4 attached.

Created attachment 92967 [details] screen-shot-missing-plugin-click-mac-safari-5.0.4

Noting that r84270 requires a windowless plugin, I wondered if the webkit NPAPI test plugin could help. I read over its main.cpp, and it does support windowless operation. Some DRT debugging on chrome-mac/win confirmed that webkit sends click events to the test plugin in windowless mode ...

Created attachment 93079 [details] Patch

chromium layout trys http://build.chromium.org/p/tryserver.chromium/builders/linux_layout/builds/653 http://build.chromium.org/p/tryserver.chromium/builders/mac_layout/builds/585 http://build.chromium.org/p/tryserver.chromium/builders/win_layout/builds/798

(In reply to comment #14) > Noting that r84270 requires a windowless plugin, I wondered if the webkit NPAPI test plugin could help. See comment 1. It's unsuitable for actually testing what this test is trying to test.

(In reply to comment #12) > There's no Flash installed on the test system, so the test was clicked on the missing > plugin and testing the missing plugin code paths That should only be true on Windows; again, see comment 1. Are you only trying to fix this on Windows, or on all platforms?

(In reply to comment #17) > See comment 1. It's unsuitable for actually testing what this test is trying to test. Unsuitable because the test plugin doesn't consume click events? Test plugin events can be logged. That was added to resolve bug 11517 "REGRESSION: Flash clicks/interactivity not working properly." The test for bug 11517 is plugins/mouse-events.html -- logged plugin events seem to be output here: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=plugins%2Fmouse-events.html

"Consume" meaning "return true from NPP_HandleEvent"; i.e., tell the browser that the click should not be propagated any further. If the click is propagating in the test then the test doesn't tell us anything about whether or not focus works with a plugin that does consume (stop) click events, because the underlying web view handling could move focus. Although now that I think about it more, on the Mac the return value is generally ignored. If that's true of all ports on all platforms, I guess we could use the test plugin after all.

> Although now that I think about it more, on the Mac the return value is generally ignored. If that's true of all ports on all platforms, I guess we could use the test plugin after all. And there are good reasons for that ignorance: refer to WebPluginDelegateImpl::PlatformHandleInputEvent webplugin_delegate_impl_mac.mm http://goo.gl/WuCtv webplugin_delegate_impl_win.cc http://goo.gl/BUQ67 webplugin_delegate_impl_gtk.cc http://goo.gl/yBmQ8

> The test for bug 11517 is plugins/mouse-events.html Noticed that on the windows ports (chrome/safari), the test plugin doesn't support event logging. Filed bug 61721 about that.

Test fails on WebKit2, but there's and exclusion in place. See http://trac.webkit.org/changeset/84279

For reference, bug 57439 is similar in that the test there involve clicking on the test plugin.

Created attachment 95329 [details] Patch

Noel: Do we ignore the return value of NPP_HandleEvent on Safari Mac and Safari Win too? Stuart: If so, is switching to the test plugin OK with you?

Yes, the only reason I avoided the test plugin is that I wasn't thinking about the return value being ignored. If it's universally true that it's ignored, I don't have any problem with using the test plugin.

> Noel: Do we ignore the return value of NPP_HandleEvent on Safari Mac and Safari Win too? I don't know, but If we don't, it's a bug. Flash Win32 always returns 0 for mouse + keyboard events according to the chromium windows code. I'll go find the code and see what I can make of it ...

This test intermittently passes on Chromium Mac.

Comment on attachment 95329 [details] Patch OK.

Comment on attachment 95329 [details] Patch Rejecting attachment 95329 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej patching file LayoutTests/plugins/mouse-click-iframe-to-plugin-expected.txt patching file LayoutTests/plugins/mouse-click-iframe-to-plugin.html Hunk #1 FAILED at 1. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/plugins/mouse-click-iframe-to-plugin.html.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10733129

LayoutTest failures for Chromium are being marked WontFix. The Bug is still accessible and referenced from TestExpectations.