Bug 62512 - [GTK] Remove webkit_web_view_get_selected_text
Summary: [GTK] Remove webkit_web_view_get_selected_text
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xan Lopez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-12 09:05 PDT by Xan Lopez
Modified: 2011-06-12 16:36 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.35 KB, patch)
2011-06-12 09:10 PDT, Xan Lopez
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 2011-06-12 09:05:47 PDT
It's unused *and* private *and* can what it does can be done with the DOM bindings, so we can just nuke it.
Comment 1 Xan Lopez 2011-06-12 09:10:02 PDT
Created attachment 96881 [details]
Patch
Comment 2 Andreas Kling 2011-06-12 09:19:19 PDT
Comment on attachment 96881 [details]
Patch

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

I'm not qualified to make decisions about the Gtk API, but it looks like a reasonable change to me.

Note that the Midori browser currently uses this API, so it would be nice of you to let them know it's being removed. :)

> Source/WebKit/gtk/ChangeLog:8
> +        It's no longer used and is a privated method, so it can be

Typo, private.
Comment 3 Martin Robinson 2011-06-12 15:50:32 PDT
:/ I guess if Midori is using it we should make it deprecated with a note to use the DOM APIs and then remove it in some future release.
Comment 4 Xan Lopez 2011-06-12 16:10:22 PDT
(In reply to comment #3)
> :/ I guess if Midori is using it we should make it deprecated with a note to use the DOM APIs and then remove it in some future release.

Maybe it was not clear in the first comment, but this method is *private*. It's not exported in any public header, so whoever is using it does it at his own risk.
Comment 5 Martin Robinson 2011-06-12 16:17:45 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > :/ I guess if Midori is using it we should make it deprecated with a note to use the DOM APIs and then remove it in some future release.
> 
> Maybe it was not clear in the first comment, but this method is *private*. It's not exported in any public header, so whoever is using it does it at his own risk.

Apologies, I was thrown off momentarily because it had gtkdoc and Midori and claws are using it. I thought perhaps you had made a mistake in thinking it private. I don't see it on the documentation site though, so I believe that you are correct that it is safe to remove. 

I feel that we should not use gtkdoc for private methods. It seems too easy for them slip into the public documentation and sends the wrong message downstream. CCing Christian so that he has some advance warning.
Comment 6 Xan Lopez 2011-06-12 16:23:21 PDT
(In reply to comment #5)
> Apologies, I was thrown off momentarily because it had gtkdoc and Midori and claws are using it. I thought perhaps you had made a mistake in thinking it private. I don't see it on the documentation site though, so I believe that you are correct that it is safe to remove. 

Maybe this is because it was used at some point in DRT (and that's the reason why it's in a header and marked with WEBKIT_API too).

> 
> I feel that we should not use gtkdoc for private methods. It seems too easy for them slip into the public documentation and sends the wrong message downstream. CCing Christian so that he has some advance warning.

Right.

FWIW, we just cannot remove public methods. What you suggested (deprecate + remove later) is what has to happen, it's not really up to debate. With private methods, of course, we can do whatever we want.
Comment 7 Xan Lopez 2011-06-12 16:36:11 PDT
Landed in r88621, closing.