RESOLVED FIXED 122551
[GTK] WebKitWebViewBase's ClickCounter should be reset
https://bugs.webkit.org/show_bug.cgi?id=122551
Summary [GTK] WebKitWebViewBase's ClickCounter should be reset
Simon Pena
Reported 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.
Attachments
Patch (2.92 KB, patch)
2013-12-12 10:18 PST, Enrique Ocaña
no flags
Patch (2.94 KB, patch)
2013-12-12 11:04 PST, Enrique Ocaña
no flags
Alexey Proskuryakov
Comment 1 2013-10-10 09:23:15 PDT
In WebProcess at least, the canonical document change time is when a load is committed.
Enrique Ocaña
Comment 2 2013-12-12 10:18:05 PST
Enrique Ocaña
Comment 3 2013-12-12 11:04:39 PST
WebKit Commit Bot
Comment 4 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
Martin Robinson
Comment 5 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?
Simon Pena
Comment 6 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 :)
Enrique Ocaña
Comment 7 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)
Martin Robinson
Comment 8 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.
Enrique Ocaña
Comment 9 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
WebKit Commit Bot
Comment 10 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>
WebKit Commit Bot
Comment 11 2013-12-20 11:42:02 PST
All reviewed patches have been landed. Closing bug.
Martin Robinson
Comment 12 2014-04-08 08:27:23 PDT
*** Bug 120401 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.