[webkit-reviews] review denied: [Bug 40641] [Qt] Clicking button input does not give it focus : [Attachment 58831] Fix for the html elements not focused by mouse.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jul 11 21:13:09 PDT 2010


Antonio Gomes <tonikitoo at webkit.org> has denied Viatcheslav Ostapenko
<ostapenko.viatcheslav at nokia.com>'s request for review:
Bug 40641: [Qt] Clicking button input does not give it focus
https://bugs.webkit.org/show_bug.cgi?id=40641

Attachment 58831: Fix for the html elements not focused by mouse.
https://bugs.webkit.org/attachment.cgi?id=58831&action=review

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
Hi Viatcheslav. As I said before, patch looks Ok to me. It is a shame that it
had to be spun off from bug 22261, where a more general solution was about to
take place, but it does not sound as a big deal to me. I look forward for a
morecross-port change involving this behavior, but we can make it happen for Qt
earlier, yes.

About the patch I'll r- is mainly for two reasons:

- you are adding your own 'log' function. Please use debug function available
at LayoutTests/fast/js/resources/js-test-pre.js

- you are adding expected results for Qt only, but not skipping the added test
for any other platform (e.g. mac, gtk, chromium). My advice is:

1)  the results should be the same for Qt and Gtk, so please add expected
results for Gtk as well, equal to Qt's one;

2) Also make your output such that it is possible to add a general failing
result for all other ports where the behavior will remain as is today.
At least for Mac, it seems like the behavior they like to have according to ap:
https://bugs.webkit.org/show_bug.cgi?id=22261#c17

The rest of the patch looks fine.


More information about the webkit-reviews mailing list