Bug 31759

Summary: [GTK] WebKitWebView needs API to scroll to selection
Product: WebKit Reporter: Peter Bloomfield <pbloomfield>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Enhancement CC: bugs-noreply, christian, darin, gustavo, mathstuf, mrobinson, pbloomfield, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
implement webkit_web_view_get_selection_bounds
none
test-case
none
more careful patch
none
patch w/ChangeLog entry
eric: review-
test program using JavaScript none

Description Peter Bloomfield 2009-11-20 18:53:33 PST
WebKitWebView API supports searching the WebView for a string, but if the WebView is not contained directly in a GtkScrolledWindow, the found string is selected, but not scrolled into view.  The use-case is a WebKitWebView packed in a GtkVBox, which is in turn contained in a GtkScrolledWindow.  One fix is to expose the bounds of the found, and selected, text.

Following patch implements:

void webkit_web_view_get_selection_bounds(WebKitWebView *web_view, GdkRectangle *selection_bounds);

and the next attachment is a test-case showing its use.
Comment 1 Peter Bloomfield 2009-11-20 18:55:40 PST
Created attachment 43636 [details]
implement webkit_web_view_get_selection_bounds
Comment 2 Peter Bloomfield 2009-11-20 18:57:03 PST
Created attachment 43637 [details]
test-case
Comment 3 Peter Bloomfield 2009-11-22 09:10:03 PST
Created attachment 43680 [details]
more careful patch
Comment 4 Peter Bloomfield 2009-11-22 15:53:13 PST
Created attachment 43689 [details]
patch w/ChangeLog entry
Comment 5 Eric Seidel (no email) 2009-11-29 13:13:15 PST
Comment on attachment 43689 [details]
patch w/ChangeLog entry

Can't you already do this via the DOM?
window.getSelection().getRangeAt(0).getBoundingClientRect()

I assume that gtk webkit exposes some sort of DOM bindings, no?  Please feel free to r? this again if I'm wrong.
Comment 6 Peter Bloomfield 2009-11-29 17:46:03 PST
(In reply to comment #5)
> (From update of attachment 43689 [details])
> Can't you already do this via the DOM?
> window.getSelection().getRangeAt(0).getBoundingClientRect()
> 
> I assume that gtk webkit exposes some sort of DOM bindings, no?  Please feel
> free to r? this again if I'm wrong.

That would be pretty cool!  I'd much rather do this within the existing API if possible--perhaps one of the WebKitGtk devs could jump in.  I'm new to WebKit, and basically working from <URL:http://webkitgtk.org/reference/index.html>, and I don't see anything that provides that functionality--but I could easily have missed it.

If something is planned, I believe we could wait for it, and use the more general solution instead of pushing for piecemeal fixes like the present patch.

But if not, I guess we have no alternative :(
Comment 7 Gustavo Noronha (kov) 2009-12-02 10:10:40 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 43689 [details] [details])
> > Can't you already do this via the DOM?
> > window.getSelection().getRangeAt(0).getBoundingClientRect()
> > 
> > I assume that gtk webkit exposes some sort of DOM bindings, no?  Please feel
> > free to r? this again if I'm wrong.
> 
> That would be pretty cool!  I'd much rather do this within the existing API if
> possible--perhaps one of the WebKitGtk devs could jump in.  I'm new to WebKit,
> and basically working from <URL:http://webkitgtk.org/reference/index.html>, and
> I don't see anything that provides that functionality--but I could easily have
> missed it.

We currently do not have DOM bindings exposed. I am not sure about the proposed API, either, so I would like to get someone who actually knows what they're talking about in this area (like Xan) gives their opinion.

> If something is planned, I believe we could wait for it, and use the more
> general solution instead of pushing for piecemeal fixes like the present patch.

We do have plans of having DOM bindings, but they need a lot of work to happen.
Comment 8 Peter Bloomfield 2009-12-02 14:57:17 PST
(In reply to comment #7)
> We currently do not have DOM bindings exposed. I am not sure about the proposed
> API, either, so I would like to get someone who actually knows what they're
> talking about in this area (like Xan) gives their opinion.
> 
> > If something is planned, I believe we could wait for it, and use the more
> > general solution instead of pushing for piecemeal fixes like the present patch.
> 
> We do have plans of having DOM bindings, but they need a lot of work to happen.

Thanks for the background!

BTW: another useful hook would be to clear the current selection.  Currently, if you search for some string, then for the empty string, the latter search fails but leaves the former match selected.  When the string is entered in a GtkEntry, and you delete one character at a time, the selection progressively shrinks to a single character, which is then left selected when the last character in the GtkEntry is deleted.  You can see this in, for example, Epiphany.  Leaving that stray character highlighted is inconsistent with the empty GtkEntry (imho).  Perhaps webkit_web_view_search_text() should be patched to do this, or perhaps findString(), but either of those would change the behavior for others.  Implementing something like

void webkit_web_view_clear_selection(WebKitWebView* webView);

would give the app the option to clean up the display.
Comment 9 Peter Bloomfield 2009-12-02 15:57:50 PST
(In reply to comment #8)
> BTW: another useful hook would be to clear the current selection.

Oops--just noticed webkit_web_view_unmark_text_matches()--sorry for the noise.
Comment 10 Peter Bloomfield 2009-12-02 16:09:35 PST
(In reply to comment #9)
> (In reply to comment #8)
> > BTW: another useful hook would be to clear the current selection.
> 
> Oops--just noticed webkit_web_view_unmark_text_matches()--sorry for the noise.

Oh, wait...it doesn't unmark text marked by webkit_web_view_search_text--is that a bug, or a feature?  OK, so as it stands, we still need webkit_web_view_clear_selection.
Comment 11 Peter Bloomfield 2010-01-10 08:13:03 PST
ping...

Any thoughts about the API?

WebKitWebView is already providing better HTML message viewing than the other widgets.  We're working on user-controllable downloading of images, for instance, which is a serious user-privacy issue.

A usable search capability would really top it off nicely!
Comment 12 Gustavo Noronha (kov) 2010-01-10 16:49:51 PST
(In reply to comment #11)
> ping...
> 
> Any thoughts about the API?

The proposed API looks sane to me, but I'd prefer to have opinions from Martin Robinson and Xan Lopez, who worked more in text selection related patches.
Comment 13 Xan Lopez 2010-01-11 04:09:39 PST
(In reply to comment #5)
> (From update of attachment 43689 [details])
> Can't you already do this via the DOM?
> window.getSelection().getRangeAt(0).getBoundingClientRect()
> 
> I assume that gtk webkit exposes some sort of DOM bindings, no?  Please feel
> free to r? this again if I'm wrong.

window.getSelection seems to be marked as JavaScript only in the IDL file though, so it wouldn't be accessible through GTK+ DOM bindings. Is there another way to do this?
Comment 14 Xan Lopez 2010-01-11 04:13:20 PST
(In reply to comment #13)
> (In reply to comment #5)
> > (From update of attachment 43689 [details] [details])
> > Can't you already do this via the DOM?
> > window.getSelection().getRangeAt(0).getBoundingClientRect()
> > 
> > I assume that gtk webkit exposes some sort of DOM bindings, no?  Please feel
> > free to r? this again if I'm wrong.
> 
> window.getSelection seems to be marked as JavaScript only in the IDL file
> though, so it wouldn't be accessible through GTK+ DOM bindings. Is there
> another way to do this?

Answering myself, apparently document.defaultView.getSelection() should do the same thing.
Comment 15 Peter Bloomfield 2010-01-11 05:20:03 PST
So from the perspective of the WebKitGtk-based app:

- the current API offers no way to find the bounds of the current selection;

- the API proposed in this bug is unacceptable because at some point the addition of DOM bindings to the WebKitGtk API will offer a cleaner solution (and, of course, a much richer API).

Is that a correct summary?  If so, is there anything I can do to help move things forward?  Is there a roadmap somewhere for implementing DOM bindings?
Comment 16 Martin Robinson 2010-01-11 09:07:57 PST
If the information you want it accessible via the window object in JavaScript, you could possibly use the JavaScriptCore API to accomplish the same thing. You might look at how Epiphany currently handles saving form data for an example.
Comment 17 Peter Bloomfield 2010-01-11 13:52:19 PST
(In reply to comment #16)
> If the information you want it accessible via the window object in JavaScript,
> you could possibly use the JavaScriptCore API to accomplish the same thing. You
> might look at how Epiphany currently handles saving form data for an example.

Thanks for the hint!  JSEvaluateScript does the trick: as Eric pointed out, window.getSelection().getRangeAt(0).getBoundingClientRect() gets the rectangle--I just didn't know that I could call it!  Similarly, window.getSelection().removeAllRanges() clears the selection.

So my immediate need is met.  It's not the most transparent API, however...
Comment 18 Peter Bloomfield 2010-01-13 08:53:47 PST
Created attachment 46469 [details]
test program using JavaScript

Attached is a slightly different test program in C, with JavaScript implementations of get_selection_bounds and clear_selection.

Is it a correct use of the JavaScript API?

Does it look like a reasonable work-around?