Bug 46723 - webkit_web_navigation_action_get_button() wrong
Summary: webkit_web_navigation_action_get_button() wrong
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Minor
Assignee: Nobody
URL: http://webkitgtk.org/reference/webkit...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 07:48 PDT by prof.maad
Modified: 2010-10-19 00:58 PDT (History)
4 users (show)

See Also:


Attachments
Patch to correct documentation for webkit_web_navigation_action_get_button() (556 bytes, patch)
2010-10-15 04:31 PDT, prof.maad
xan.lopez: review-
Details | Formatted Diff | Diff
Patch to correct documentation for webkit_web_navigation_action_get_button() (2.02 KB, patch)
2010-10-15 06:00 PDT, prof.maad
no flags Details | Formatted Diff | Diff
Patch to correct documentation for webkit_web_navigation_action_get_button() (with style correction) (2.02 KB, patch)
2010-10-15 06:05 PDT, prof.maad
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description prof.maad 2010-09-28 07:48:53 PDT
The documentation in WebKitGTK webkit_web_navigation_action_get_button() states that it returns the following buttons:

0 - left mouse button
1 - middle mouse button
2 - right mouse button

This is plain wrong, it returns:

1 - left mouse button
2 - middle mouse button
3 - right mouse button

This was probably introduced in this bug report: https://bugs.webkit.org/show_bug.cgi?id=25555
Comment 1 Xan Lopez 2010-10-15 04:25:41 PDT
You are right, we add + 1 to use the GTK+ button values. Care to make a patch correcting the doc?
Comment 2 prof.maad 2010-10-15 04:31:47 PDT
Created attachment 70849 [details]
Patch to correct documentation for webkit_web_navigation_action_get_button()

This patch corrects the documentation for webkit_web_navigation_action_get_button() to specify the right button codes.
Comment 3 prof.maad 2010-10-15 04:32:29 PDT
Please be gentle, this is the very first patch I submitted to a Gtk project, so I had basically no idea what I was doing.
Comment 4 Xan Lopez 2010-10-15 04:38:19 PDT
Comment on attachment 70849 [details]
Patch to correct documentation for webkit_web_navigation_action_get_button()

Some comments:

- The text appears twice in the file, you need to fix it in both places.
- Your fix basically is telling a lie, since those are not the DOM values, but the GTK+ ones. I think we should either tell the whole story, or just say that the identifier for the clicked button is returned, leaving it implicit that it's GTK+'s. I'd vote to do the former.
- You need to generate a ChangeLog and set the patch as r? when submitting it. Go over this document (http://webkit.org/coding/contributing.html) quickly, but at the very least I need the ChangeLog to commit your patch :)

Thanks!
Comment 5 prof.maad 2010-10-15 06:00:41 PDT
Created attachment 70859 [details]
Patch to correct documentation for webkit_web_navigation_action_get_button()

Adapted the patch to the WebKit guidelines and added a Changelog entry.
Comment 6 WebKit Review Bot 2010-10-15 06:03:08 PDT
Attachment 70859 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 prof.maad 2010-10-15 06:05:55 PDT
Created attachment 70861 [details]
Patch to correct documentation for webkit_web_navigation_action_get_button() (with style correction)

Corrected style error (tab in ChangeLog).
Comment 8 Xan Lopez 2010-10-19 00:45:31 PDT
Comment on attachment 70861 [details]
Patch to correct documentation for webkit_web_navigation_action_get_button() (with style correction)

Looks great, thank you.
Comment 9 WebKit Commit Bot 2010-10-19 00:58:48 PDT
Comment on attachment 70861 [details]
Patch to correct documentation for webkit_web_navigation_action_get_button() (with style correction)

Clearing flags on attachment: 70861

Committed r70035: <http://trac.webkit.org/changeset/70035>
Comment 10 WebKit Commit Bot 2010-10-19 00:58:54 PDT
All reviewed patches have been landed.  Closing bug.