Add -[WebView scrollDOMRangeToVisible:]

• Component: HTML Editing
• Status: RESOLVED FIXED

[Duncan Wilcox]

-[WebView setSelectedDOMRange:affinity:] calls -[WebCoreBridge 
setSelectedDOMRange:affinity:closeTyping:], that in turn doesn't appear to ever directly or indirectly call -
[WebCoreBridge ensureSelectionVisible].

-[WebView setSelectedDOMRange:affinity:] isn't documented as making the selection visible, but if it turns 
out it shouldn't then there's no public API for functionality equivalent to -[WebCoreBridge 
ensureSelectionVisible].

[Duncan Wilcox]

Also javascript code seems to follow a completely different path and doesn't appear to scroll the view (as 
far as I could tell browsing through the code and writing small tests).

[Joost de Valk (AlthA)]

Is there any way to reproduce this? A layout test perhaps? An automated webkit test? As long as there 
isn't, i can't confirm this bug.

[Duncan Wilcox]

Created attachment 2725 [details]
testcase

Here's an objective-c testcase. Create a cocoa app in xcode, replace main.m
with the attachment and add the WeKit.framework to the project.

[Duncan Wilcox]

Created attachment 2726 [details]
js testcase

Here's a javascript testcase, same effect (doesn't scroll to selection), though
the codepath it follows internally is completely different.

[Darin Adler]

I don't think that changing the selection should have a side effect of scrolling to the selection.

On the other hand, WebView's API for selection and editing was based on NSTextView's API, and 
NSTextView has a scrollRangeToVisible: method. We might want to add the equivalent.

[Justin Garcia]

Created attachment 4235 [details]
Patch

Added -[WebView scrollDOMRangeToVisible:] to public-pending api.

For the bridge method, I look for a node in the DOM range that has a valid
rect, in case the start/end containers are invisible.

-[NSText scrollRangeToVisible:]'s documentation
<http://tuvix.apple.com/documentation/Cocoa/Reference/ApplicationKit/ObjC_classic/Classes/NSText.html#//apple_ref/doc/uid/20000367-scrollRangeToVisible_>
reads:
Scrolls the receiver in its enclosing scroll view so the first characters of
aRange are visible.

This isn't clear on where the range will appear within the scrolled view.  I
just put it in the upper left.

Tested with the previously attached sample app.

[Justin Garcia]

Oops I just realized that -[WebCoreBridge firstRectForDOMRange] already exists.  I'll redo this to use that.

[David Smith]

Created attachment 10750 [details]
Updated patch

I noticed this bug hadn't been touched in a while, so I updated the patch to a) work on the modern directory layout/file names, and b) use -[WebCoreBridge firstRectForDOMRange] as suggested by Justin.

[David Smith]

Created attachment 10773 [details]
Updated patch take 2: no tabs here

Silly Xcode not being able to set tabs/spaces per-project.

[Timothy Hatcher]

Comment on attachment 10773 [details]
Updated patch take 2: no tabs here

Sorry still has tabs.

Other comments:

You need to add this to WebViewPrivate.h. We can't modify WebView.h wihtout getting the API approved first.

Please include a ChangeLog.

[Timothy Hatcher]

You should also add yourself to the copyright in the header of WebView.m and WebCoreFrameBridge.m

[David Smith]

Created attachment 10779 [details]
Updated patch, take 3

Doublechecked the tabs (regex search in Xcode for \t should catch all of them, right?), added changelog entry, moved declaration to WebViewPrivate, fixed a spacing issue to conform to coding style, added self to WebView.m and WebCoreFrameBridge.mm copyright section.

[Mark Rowe (bdash)]

Landed in r16761.