Bug 114980 – [GTK] Fix remaining introspection warnings

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

There are a few introspection warnings. We should fix them.

[Martin Robinson]

Created attachment 199089 [details]
Patch

[WebKit Commit Bot]

Attachment 199089 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/gobject/WebKitDOMCustom.h', u'Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h', u'Source/WebKit/gtk/ChangeLog', u'Source/WebKit/gtk/webkit/webkitspellchecker.h', u'Source/WebKit/gtk/webkit/webkitversion.h.in']" exit_code: 1
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:47:  Extra space between gpointer and user_data  [whitespace/declaration] [3]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:47:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:47:  user_data is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:49:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:71:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:74:  Extra space between gpointer and user_data  [whitespace/declaration] [3]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:74:  user_data is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:85:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/gobject/WebKitDOMCustom.h:39:  The parameter name "input" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/gobject/WebKitDOMCustom.h:117:  The parameter name "flow" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/gobject/WebKitDOMCustom.h:127:  The parameter name "flow" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 12 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

(In reply to comment #2)
> Attachment 199089 [details] did not pass style-queue:

I believe these warnings are due to the fact that WebKitDOMEventTarget.h (which is a public header) is not skipped by the style bot.

[Gustavo Noronha (kov)]

Comment on attachment 199089 [details]
Patch

OK

[Martin Robinson]

Comment on attachment 199089 [details]
Patch

Clearing flags on attachment: 199089

Committed r148914: <http://trac.webkit.org/changeset/148914>

[Martin Robinson]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

Comment on attachment 199089 [details]
Patch

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

I think you could have added the custom bindings headers to the list of files ignored by the style checker.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:32
> +/**
> + * webkit_dom_html_text_area_element_is_edited:
> + * @input: A #WebKitDOMHTMLTextAreaElement
> + *
> + */
> +WEBKIT_API gboolean webkit_dom_html_text_area_element_is_edited(WebKitDOMHTMLTextAreaElement* input);

Doesn't this cause a warning? A method returning a boolean without a Returns: tag?

>> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:39
>> +WEBKIT_API gboolean webkit_dom_html_input_element_is_edited(WebKitDOMHTMLInputElement* input);
> 
> The parameter name "input" adds no information, so it should be removed.  [readability/parameter_name] [5]

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:-30
> -/* Compatibility */

Why removing this? If we add the deprecated tag to deprecated methods, this comment becomes redundant, otherwise I think it's useful.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:49
> + * Returns: (transfer none):
> + */

Now that we have doc comments here we could add the deprecated tag.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:57
> + *

Why this extra line here?

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:65
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:74
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:81
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:88
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:97
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:106
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:115
> + *

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:125
> + *

Ditto.

[Martin Robinson]

(In reply to comment #7)
> (From update of attachment 199089 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=199089&action=review
> 
> I think you could have added the custom bindings headers to the list of files ignored by the style checker.

It's best handled in another bug.

> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:32
> > +/**
> > + * webkit_dom_html_text_area_element_is_edited:
> > + * @input: A #WebKitDOMHTMLTextAreaElement
> > + *
> > + */
> > +WEBKIT_API gboolean webkit_dom_html_text_area_element_is_edited(WebKitDOMHTMLTextAreaElement* input);
> 
> Doesn't this cause a warning? A method returning a boolean without a Returns: tag?

No. gbooleans aren't transferred and have no scope so they need neither the scope nor the transfer annotation.

> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:-30
> > -/* Compatibility */
> 
> Why removing this? If we add the deprecated tag to deprecated methods, this comment becomes redundant, otherwise I think it's useful.
> 
> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:49
> > + * Returns: (transfer none):
> > + */

It didn't seem to be adding much information. git blame is sufficient for explaining why they are there. I'm happy to restore it though, if you think it's severe enough.

> Now that we have doc comments here we could add the deprecated tag.

Sure.

> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:57
> > + *
> 
> Why this extra line here?

Extra line nits a day after the patch has landed? :) You are welcome to fix them and any other issues you see with the gtkdoc and I'll be happy to review without delay. Another issue with the style is that the C++ file isn't in WebKit style.

[C Anthony]

this patch doesn't fix anything introspection related with regards to add/remove_event_listener, as painstakingly detailed by this entirety bug:

https://bugs.webkit.org/show_bug.cgi?id=77835
https://bugs.webkit.org/show_bug.cgi?id=77835#c12
https://bugs.webkit.org/show_bug.cgi?id=77835#c16
https://bugs.webkit.org/show_bug.cgi?id=77835#c22
https://bugs.webkit.org/show_bug.cgi?id=77835#c45
https://bugs.webkit.org/show_bug.cgi?id=77835#c49
https://bugs.webkit.org/show_bug.cgi?id=77835#c51

...i have had WORKING PATCHES IN QUEUE for almost 2 years now! when, when, when can this be fixed properly? WebKit7? seriously, who do i have to beg to make it happen? i was given the impression that it might "[...] go in 1.8.1 since it's pretty important for bindings", but here we are now at WebKit-next-next-next... to be perfectly clear:

THE LACK OF add/remove_event_listener() IN INTROSPECTED BINDINGS RENDERS WEBKIT WORSE THAN USELESS FOR *EVERYONE* OTHER THAN C.

please, please, revisit this, or revert this patch, because now the problem looks "fixed" when it very clearly is not. `async` is INCORRECT transfer. and even if it did work, remove_event_listener() won't work with it anyway (the reasons are detailed in the links).

people are working around this is disgusting ways:

http://stackoverflow.com/questions/17322872/cant-find-domeventtarget-add-event-listener-in-python

...can i get some closure here please? i think i've gone WAY ABOVE AND BEYOND trying to get this damn thing merged, and at this point (2... YEARS), i'm pretty irritated from being BLOWN OFF AND BLATANTLY IGNORED.

this is a REAL problem. please LET ME SOLVE IT. *now*.

thank you.

[C Anthony]

apologize for duplicate emails, i didn't realize i could add multiple people with commas (and prob spaces now that i think of it).

i'm also sorry if i appear crass or whatever, but i honestly don't know what more i can do short of physically walking to Apple and picketing outside.  if it seems like i'm YELLING, it's because I AM SHOUTING OBSCENITIES AS I TYPE THIS.

c'mon guys, i did all the work 3 times over and implemented in multiple ways, just pick one. any one. i fully intend on setting a reminder to ping both of these bugs until i can get a concrete path forward...

i don't need an immediate resolution, but you guys could throw me a bone and give me some glimmer of hope by simply ACKNOWLEDGING THE PROBLEM *OFFICIALLY* BY ACCEPTING THIS BUG:

https://bugs.webkit.org/show_bug.cgi?id=77835

thanks.