Bug 122551 – [GTK] WebKitWebViewBase's ClickCounter should be reset

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Simon Pena]

(From bug #120401) The ClickCounter helper class is used to handle single-clicks, double-clicks, triple-clicks and so on: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/gtk/GtkClickCounter.h

However, it is not being reset appropriately, and under some circumstances it could report an incorrect number of clicks (an example of this scenario is the original bug, where it affected the layout tests). It should be reset, but exactly when is still unclear: see https://bugs.webkit.org/show_bug.cgi?id=120401#c3, where Martin says:

> Maybe a better time would be after the document changes. It might be hard to track this in the UI process, so perhaps it's better to wait for the first repaint that includes the new page. This should probably also be in WebKitWebViewBase.

[Alexey Proskuryakov]

In WebProcess at least, the canonical document change time is when a load is committed.

[Enrique Ocaña]

Created attachment 219092 [details]
Patch

[Enrique Ocaña]

Created attachment 219098 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Martin Robinson]

This is better though it doesn't handle loads in subframes. That's probably okay since this situation has only ever been seen during test runs. Can you confirm that the tests listed in the original bug (120401) are passing now?

[Simon Pena]

Thanks for taking over! And sorry for not having been able to follow up on the previous patch!

It would be great if you could also take a look at bug #120458 after you are done with this one :)

[Enrique Ocaña]

(In reply to comment #5)
> This is better though it doesn't handle loads in subframes. That's probably okay since this situation has only ever been seen during test runs. Can you confirm that the tests listed in the original bug (120401) are passing now?

I executed Simón's command in bug #120401 and these are the results:

- Without the patch: 55 tests ran as expected, 29 didn't.
- With the patch: 76 tests ran as expected, 8 didn't.

The failing tests are:

[1/4] editing/selection/hit-test-on-text-with-line-height.html failed unexpectedly (test timed out)
[2/4] editing/selection/doubleclick-beside-cr-span.html failed unexpectedly (test timed out)
[3/4] editing/selection/rtl-move-selection-right-left.html failed unexpectedly (test timed out)
[4/4] editing/selection/move-begin-end.html failed unexpectedly (text diff)

[Martin Robinson]

(In reply to comment #7)
> (In reply to comment #5)
> > This is better though it doesn't handle loads in subframes. That's probably okay since this situation has only ever been seen during test runs. Can you confirm that the tests listed in the original bug (120401) are passing now?
> 
> I executed Simón's command in bug #120401 and these are the results:
> 
> - Without the patch: 55 tests ran as expected, 29 didn't.
> - With the patch: 76 tests ran as expected, 8 didn't.
> 
> The failing tests are:
> 
> [1/4] editing/selection/hit-test-on-text-with-line-height.html failed unexpectedly (test timed out)
> [2/4] editing/selection/doubleclick-beside-cr-span.html failed unexpectedly (test timed out)
> [3/4] editing/selection/rtl-move-selection-right-left.html failed unexpectedly (test timed out)
> [4/4] editing/selection/move-begin-end.html failed unexpectedly (text diff)

Are any of the tests that use to fail skipped for the GTK+? If so perhaps they can now be unskipped.

[Enrique Ocaña]

No, they are only skipped for wincairo, mac-wk2 and win, not for gtk{,-wk1,wk2}:

$ cat /tmp/nowpass.txt
editing/selection/anchor-focus1.html
editing/selection/anchor-focus2.html
editing/selection/anchor-focus3.html
editing/selection/block-with-positioned-lastchild.html
editing/selection/doubleclick-crash.html
editing/selection/drag-select-1.html
editing/selection/editable-links.html
editing/selection/expanding-selections.html
editing/selection/expanding-selections2.html
editing/selection/fake-doubleclick.html
editing/selection/inline-closest-leaf-child.html
editing/selection/mixed-editability-1.html
editing/selection/paragraph-granularity.html
editing/selection/select-across-readonly-input-1.html
editing/selection/select-across-readonly-input-2.html
editing/selection/select-across-readonly-input-3.html
editing/selection/select-across-readonly-input-4.html
editing/selection/select-across-readonly-input-5.html
editing/selection/select-all-iframe.html
editing/selection/select-from-textfield-outwards.html
editing/selection/selection-actions.html

$ for i in `find LayoutTests -name TestExpectations`; do for j in `cat /tmp/nowpass.txt`; do grep -Hn $j $i; done; done
LayoutTests/platform/wincairo/TestExpectations:2682:editing/selection/select-from-textfield-outwards.html
LayoutTests/platform/mac-wk2/TestExpectations:152:editing/selection/select-across-readonly-input-2.html
LayoutTests/platform/mac-wk2/TestExpectations:153:editing/selection/select-across-readonly-input-3.html
LayoutTests/platform/mac-wk2/TestExpectations:154:editing/selection/select-across-readonly-input-4.html
LayoutTests/platform/mac-wk2/TestExpectations:155:editing/selection/select-across-readonly-input-5.html
LayoutTests/platform/win/TestExpectations:2208:editing/selection/select-from-textfield-outwards.html

[WebKit Commit Bot]

Comment on attachment 219098 [details]
Patch

Clearing flags on attachment: 219098

Committed r160918: <http://trac.webkit.org/changeset/160918>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Martin Robinson]

*** Bug 120401 has been marked as a duplicate of this bug. ***