fail: --- /tmp/layout-test-results/fast/notifications/notifications-click-event-expected.txt 2011-11-07 16:35:36.762379664 +0000 +++ /tmp/layout-test-results/fast/notifications/notifications-click-event-actual.txt 2011-11-07 16:35:36.762379664 +0000 @@ -1,8 +1,6 @@ DESKTOP NOTIFICATION: icon , title New E-mail, text Meet me tonight at 8! -DESKTOP NOTIFICATION CLOSED: New E-mail Showing notifications. To exercise manually, grant notification permissions and load this page, then click on the notification. You should see a "PASS" message. -PASS: click event fired.

One more culprit test which makes the other fail: $ Tools/Scripts/old-run-webkit-tests --wait-for-httpd http/tests/notifications/icon-exists-show-alert-during-load.html fast/notifications/notifications-click-event.html

Created attachment 121094 [details] proposed fix This patch clear the m_notifications in the resetToConsistentStateBeforeTesting() so the notification tests don't cause side effect in the next notification test

Your patch makes sense, but I'm curious about one bit: Why do the other ports not need this? Where do _they_ reset the state of their notification presenter? AFAICS disconnectFrame() only resets permissions, but does that imply cancellation of any visible notifications, too?

(In reply to comment #4) > Your patch makes sense, but I'm curious about one bit: Why do the other ports not need this? Where do _they_ reset the state of their notification presenter? I have to look after it how it works on the other platforms. This problem is caused by the m_notifications which is still containing the notifications from the previous tests when the flaky one is running so the "click" sometimes goes to the wrong notification. > > AFAICS disconnectFrame() only resets permissions, but does that imply cancellation of any visible notifications, too? I don't know, I will check this too.

(In reply to comment #5) > (In reply to comment #4) > > Your patch makes sense, but I'm curious about one bit: Why do the other ports not need this? Where do _they_ reset the state of their notification presenter? > > I have to look after it how it works on the other platforms. > This problem is caused by the m_notifications which is still containing the notifications from the previous tests when the flaky one is running so the "click" sometimes goes to the wrong notification. > > > > > AFAICS disconnectFrame() only resets permissions, but does that imply cancellation of any visible notifications, too? > > I don't know, I will check this too. I looked after it in the chromium, and I found this in the changelog: + Adds necessary hooks to chromium's DRT so that clicks on desktop notifications + can be simulated during a layout test. Requires storing a list of active + notifications so that they can be referred to later for clicking. + https://bugs.webkit.org/show_bug.cgi?id=44800 That means it is normal to the previous test's notification is still reachable btw there isn't any tests what work this way and this can cause sideeffects like this one.

(In reply to comment #4) > Your patch makes sense, but I'm curious about one bit: Why do the other ports not need this? Where do _they_ reset the state of their notification presenter? > Other ports do not run http/notifications tests, so they don't have this problem.

Comment on attachment 121094 [details] proposed fix This patch makes sense to me. We identify notifications by their title, but all of the test notifications use the same title. That's what causes the side effect. IMO it is ok to clear the notifications between tests, even if in the real world we would not cancel notifications when navigating away from the page that created them.

Comment on attachment 121094 [details] proposed fix Rejecting attachment 121094 [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: ines). patching file Source/WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.cpp patching file Source/WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.h patching file Tools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp Hunk #1 succeeded at 555 (offset -4 lines). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Simon Haus..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11856198

Arghhhh ... why wasn't it committed? Kristóf? Otherwise http/tests/notifications tests were removed from trunk - https://trac.webkit.org/changeset/115153 Do we still need this fix?

ping?

notifications and Qt isn't in WebKit trunk long time ago.