Bug 122551 - [GTK] WebKitWebViewBase's ClickCounter should be reset
Summary: [GTK] WebKitWebViewBase's ClickCounter should be reset
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
: 120401 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-10-09 06:44 PDT by Simon Pena
Modified: 2014-04-08 08:27 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.92 KB, patch)
2013-12-12 10:18 PST, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (2.94 KB, patch)
2013-12-12 11:04 PST, Enrique Ocaña
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pena 2013-10-09 06:44:27 PDT
(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.
Comment 1 Alexey Proskuryakov 2013-10-10 09:23:15 PDT
In WebProcess at least, the canonical document change time is when a load is committed.
Comment 2 Enrique Ocaña 2013-12-12 10:18:05 PST
Created attachment 219092 [details]
Patch
Comment 3 Enrique Ocaña 2013-12-12 11:04:39 PST
Created attachment 219098 [details]
Patch
Comment 4 WebKit Commit Bot 2013-12-12 11:05:08 PST
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
Comment 5 Martin Robinson 2013-12-12 14:49:00 PST
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?
Comment 6 Simon Pena 2013-12-13 01:26:28 PST
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 :)
Comment 7 Enrique Ocaña 2013-12-13 12:19:54 PST
(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)
Comment 8 Martin Robinson 2013-12-16 01:01:44 PST
(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.
Comment 9 Enrique Ocaña 2013-12-19 07:01:18 PST
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
Comment 10 WebKit Commit Bot 2013-12-20 11:41:59 PST
Comment on attachment 219098 [details]
Patch

Clearing flags on attachment: 219098

Committed r160918: <http://trac.webkit.org/changeset/160918>
Comment 11 WebKit Commit Bot 2013-12-20 11:42:02 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Martin Robinson 2014-04-08 08:27:23 PDT
*** Bug 120401 has been marked as a duplicate of this bug. ***