Bug 59721 – REGRESSION (WebKit2): (Mac) Selection is gone after switching tabs

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Jon Lee]

1. open yahoo.com in tab1
2. Open tab2
3 in tab1 select a word or sentence
4. Click tab2
5 Click tab1

Selection is gone after switching tabs.

[Jon Lee]

<rdar://problem/9327332>

[Jon Lee]

Created attachment 91615 [details]
Patch

Comment on attachment 91615 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:90
> +- (id)_newFirstResponderAfterResigning;

Is this used anywhere?

[Jon Lee]

(In reply to comment #3)
> (From update of attachment 91615 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91615&action=review
> 
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:90
> > +- (id)_newFirstResponderAfterResigning;
> 
> Is this used anywhere?

Good point. I thought I needed it when I was first fixing the bug. Will remove.

[Jon Lee]

Created attachment 91619 [details]
Patch

[Darin Adler]

Comment on attachment 91619 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.h:45
> +- (BOOL)maintainsInactiveSelection;

Does this need to be in the header file? Normally, we don’t have to declare methods if we’re just overriding something in NSView. We only need to put the method in the header if it’s a new method that some other class is going to call.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:318
> +    if (![self maintainsInactiveSelection])
> +        _data->_page->clearSelection();

Is there a good reason to call maintainsInactiveSelection here? It’s just a function that always returns NO.

Do we expect people to make subclasses of WKView and override the maintainsActiveSelection method?

> Source/WebKit2/UIProcess/API/mac/WKView.mm:564
> +- (BOOL)maintainsInactiveSelection
> +{
> +    return false;
> +}

For BOOL it’s YES and NO. The values false and true are of type bool.

Comment on attachment 91619 [details]
Patch

r- for now because I think Jon is going to change this to normal WKPage API rather than WKView API (which really should not exist at this point).

[Jon Lee]

Created attachment 91717 [details]
Patch

Comment on attachment 91717 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKPage.h:349
> +WK_EXPORT void WKPageSetMaintainsInactiveSelection(WKPageRef page, bool);

In these C API headers, we usually name all parameters, including booleans. Please call this one “maintainsInactiveSelection” or something.

> Source/WebKit2/UIProcess/WebPageProxy.h:258
> +    void setMaintainsInactiveSelection(bool newValue);

No need for the name here, though!

[Jon Lee]

Committed r85356: <http://trac.webkit.org/changeset/85356>