Bug 45666 – [Qt] Click's count is limited to three continuous click

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Ragner Magalhaes]

How the click's count is limited to three continuous click
the test LayoutTests/fast/events/click-count.html is failing.

[Ragner Magalhaes]

Created attachment 67453 [details]
Proposed patch

[Ragner Magalhaes]

Comment on attachment 67453 [details]
Proposed patch

Adds a click's count on WebKit/qt/qwebpage.[h,cpp] to enable support for more than three continuous click.

[Antonio Gomes]

Comment on attachment 67453 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67453&action=review

Good patch! I like the way you took the click couting control out of EventSender class.

A few nits, questions and requests:

1) How many consecutive clicks we support now? (triple clicking used to be the limite, right?)
2) You're removing a bunch of methods from WebPagePrivate class, Please, explain in our changelog why and how it is working now.

It would be great if we could make sure that, after your change, when the routine DlbClick was suposed to be called, it clickCould would be '2'. Same for the triple clicking routine.

Any way to assert of this case as a sanity check?

> WebKit/qt/Api/qwebpage.cpp:628
> +    Frame *focusedFrame;

Make it Frame *focusedFrame = 0;

> WebKit/qt/Api/qwebpage.cpp:672
> +    Frame *focusedFrame;

Make it Frame *focusedFrame = 0;

> WebKit/qt/ChangeLog:7
> +

Add here why you are removing the double and triple click handler routines, and how it is working not.

[Antonio Gomes]

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

[Robert Hogan]

Ragner, are you going to clean up the patch for landing?

[Caio Marcelo de Oliveira Filho]

(In reply to comment #5)
> Ragner, are you going to clean up the patch for landing?

AFAIK Ragner is not working on the project anymore. (I'm guessing you want to clean it up then, right? :-)

[Hugo Parente Lima]

I have a patch to fix this bug on wk2, but it depends on bug#45666, I'll upload it when the patch for bug#45666 lands on mainline.

[Caio Marcelo de Oliveira Filho]

(In reply to comment #7)
> I have a patch to fix this bug on wk2, but it depends on bug#45666, I'll upload it when the patch for bug#45666 lands on mainline.

Hugo, maybe you made a typo here. *This* is bug 45666. :-)

[Hugo Parente Lima]

(In reply to comment #8)
> (In reply to comment #7)
> > I have a patch to fix this bug on wk2, but it depends on bug#45666, I'll upload it when the patch for bug#45666 lands on mainline.
> 
> Hugo, maybe you made a typo here. *This* is bug 45666. :-)

Then I have a self circular dependency!

The right bug is bug#73366 =]

[Hugo Parente Lima]

Created attachment 118049 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 118049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118049&action=review

> Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:162
> +    if (m_clickTimer.isActive()
> +            && m_previousClickButton == ev->button()
> +            && (ev->pos() - m_lastClick).manhattanLength() < qApp->styleHints()->startDragDistance()) {

wrong indentation according to the style guide... (hint, it is indented too much)

> Tools/ChangeLog:8
> +        Update m_time at every call to leapForward, so dbl clicks

write out "double"

[Hugo Parente Lima]

Comment on attachment 118049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118049&action=review

>> Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:162
>> +            && (ev->pos() - m_lastClick).manhattanLength() < qApp->styleHints()->startDragDistance()) {
> 
> wrong indentation according to the style guide... (hint, it is indented too much)

hmmm... check-webkit-style isn't perfect :-(

>> Tools/ChangeLog:8
>> +        Update m_time at every call to leapForward, so dbl clicks
> 
> write out "double"

Ok, fixing and re-uploading...

[Hugo Parente Lima]

Created attachment 118052 [details]
Patch

[Hugo Parente Lima]

I'm not a commiter, can someone commit this for me? :-)

Sorry for not upload the patch asking for the commit queue flag, I did learn it just minutes before sending the patch.

[WebKit Review Bot]

Comment on attachment 118052 [details]
Patch

Rejecting attachment 118052 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[Igor Trindade Oliveira]

Comment on attachment 118052 [details]
Patch

Adding to commit queue

[WebKit Review Bot]

Comment on attachment 118052 [details]
Patch

Clearing flags on attachment: 118052

Committed r102488: <http://trac.webkit.org/changeset/102488>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.