Bug 70670 - [GTK] Remove most of the documentation errors from WebKit1
Summary: [GTK] Remove most of the documentation errors from WebKit1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-21 20:54 PDT by Martin Robinson
Modified: 2011-10-27 18:41 PDT (History)
5 users (show)

See Also:


Attachments
Patch (77.40 KB, patch)
2011-10-21 20:59 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (77.37 KB, patch)
2011-10-21 23:33 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Fix issues mentioned (77.37 KB, patch)
2011-10-22 08:20 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2011-10-21 20:54:41 PDT
This bug track removing all errors from WebKit1 except for missing documentation and undocumented enums.
Comment 1 Martin Robinson 2011-10-21 20:59:00 PDT
Created attachment 112067 [details]
Patch
Comment 2 Martin Robinson 2011-10-21 21:00:44 PDT
Sorry for the large patch, but most of these changes are quite mechanical. This reduces documentation errors to < 10.
Comment 3 WebKit Review Bot 2011-10-21 21:02:12 PDT
Attachment 112067 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1

Source/WebKit/gtk/webkit/webkitspellchecker.h:48:  Extra space between int and *misspelling_location  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitspellchecker.h:49:  Extra space between int and *misspelling_length  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebplugin.h:84:  Extra space between gboolean and enabled  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:66:  Extra space between WebKitDOMNode and *node  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:70:  Extra space between gdouble and x  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:71:  Extra space between gdouble and y  [whitespace/declaration] [3]
Total errors found: 6 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Priit Laes (IRC: plaes) 2011-10-21 23:17:07 PDT
(In reply to comment #2)
> Sorry for the large patch, but most of these changes are quite mechanical. This reduces documentation errors to < 10.

Some minor issues I spotted:

Source/WebKit/gtk/webkit/webkitspellcheckerenchant.cpp: @SECTION:webkitspellcheckerenchant

Docstring contains: 'for for' 


Source/WebKit/gtk/webkit/webkitwebsettings.h: @WebKitEditingBehavior:

WEBKIT_EDITING_BEHAVIOR_UNIX description looks inconsistent due to the usage of 'The ...'
Comment 5 Martin Robinson 2011-10-21 23:33:48 PDT
Created attachment 112078 [details]
Patch
Comment 6 Martin Robinson 2011-10-21 23:34:10 PDT
Thanks for the review. I've updated the patch fixing these issues.
Comment 7 WebKit Review Bot 2011-10-21 23:36:39 PDT
Attachment 112078 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1

Source/WebKit/gtk/webkit/webkitspellchecker.h:48:  Extra space between int and *misspelling_location  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitspellchecker.h:49:  Extra space between int and *misspelling_length  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebplugin.h:84:  Extra space between gboolean and enabled  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:66:  Extra space between WebKitDOMNode and *node  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:70:  Extra space between gdouble and x  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:71:  Extra space between gdouble and y  [whitespace/declaration] [3]
Total errors found: 6 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Daniel Bates 2011-10-22 00:31:14 PDT
Comment on attachment 112078 [details]
Patch

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

This patch looks reasonable to me. That being said, it's probably best if someone familiar with the WebKit-GTK/GTK API and GTK API notation to review this patch for correctness.

> Source/WebKit/gtk/webkit/webkitspellcheckerenchant.cpp:35
> + * WebKitGTK+. It uses the enchant dictionaries installed on the system to

enchant => Enchant

> Source/WebKit/gtk/webkit/webkitwebview.cpp:3935
> + * Return value: (transfer none): the main #WebKitWebFrame for @web_view

Missing period.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4233
> + * @flag: whether or not @web_view should be trasparent

trasparent => transparent
Comment 9 Martin Robinson 2011-10-22 08:20:17 PDT
Created attachment 112090 [details]
Fix issues mentioned
Comment 10 Martin Robinson 2011-10-22 08:22:58 PDT
(In reply to comment #8)

Thanks for the review. :) I fixed all issues except:

> > Source/WebKit/gtk/webkit/webkitwebview.cpp:3935
> > + * Return value: (transfer none): the main #WebKitWebFrame for @web_view
> 
> Missing period.

GTK+ documentation is really inconsistent about this. I prefer not to leave it if there's one phrase because it's a not a complete sentence. See for instance: 

http://developer.gnome.org/gtk/2.24/GtkWidget.html#gtk-widget-send-focus-change
Comment 11 WebKit Review Bot 2011-10-22 08:25:24 PDT
Attachment 112090 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1

Source/WebKit/gtk/webkit/webkitspellchecker.h:48:  Extra space between int and *misspelling_location  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitspellchecker.h:49:  Extra space between int and *misspelling_length  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebplugin.h:84:  Extra space between gboolean and enabled  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:66:  Extra space between WebKitDOMNode and *node  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:70:  Extra space between gdouble and x  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitwebinspector.h:71:  Extra space between gdouble and y  [whitespace/declaration] [3]
Total errors found: 6 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Gustavo Noronha (kov) 2011-10-27 11:59:05 PDT
Comment on attachment 112090 [details]
Fix issues mentioned

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

gorgeous

> Source/WebKit/gtk/docs/webkitgtk-docs.sgml:134
>    <index id="index-1.3.8" role="1.3.8">
>      <title>Index of new symbols in 1.3.8</title>
>    </index>

We also need to update these to include many versions =(.

> Source/WebKit/gtk/docs/webkitgtk-sections.txt:524
> +webkit_get_icon_database
> +
> +webkit_get_text_checker
> +webkit_set_text_checker
> +

Why the empty lines?
Comment 13 Martin Robinson 2011-10-27 18:14:36 PDT
(In reply to comment #12)

Thanks for the review!

> > Source/WebKit/gtk/docs/webkitgtk-docs.sgml:134
> >    <index id="index-1.3.8" role="1.3.8">
> >      <title>Index of new symbols in 1.3.8</title>
> >    </index>
> 
> We also need to update these to include many versions =(.

Perhaps there is a way to actually conglomerate the new symbols into major stable versions the way that GTK+ does: http://developer.gnome.org/gtk3/stable/
> 
> > Source/WebKit/gtk/docs/webkitgtk-sections.txt:524
> > +webkit_get_icon_database
> > +
> > +webkit_get_text_checker
> > +webkit_set_text_checker
> > +
> 
> Why the empty lines?

I was just seperating out unrelated pieces here. I'll remove it when landing since we don't usually do that.
Comment 14 Martin Robinson 2011-10-27 18:40:57 PDT
Committed r98677: <http://trac.webkit.org/changeset/98677>