Bug 59721 - REGRESSION (WebKit2): (Mac) Selection is gone after switching tabs
Summary: REGRESSION (WebKit2): (Mac) Selection is gone after switching tabs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-04-28 11:45 PDT by Jon Lee
Modified: 2011-04-29 13:43 PDT (History)
2 users (show)

See Also:


Attachments
Patch (10.47 KB, patch)
2011-04-28 18:48 PDT, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (10.25 KB, patch)
2011-04-28 19:21 PDT, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (11.43 KB, patch)
2011-04-29 12:12 PDT, Jon Lee
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2011-04-28 11:45:24 PDT
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.
Comment 1 Jon Lee 2011-04-28 11:46:01 PDT
<rdar://problem/9327332>
Comment 2 Jon Lee 2011-04-28 18:48:45 PDT
Created attachment 91615 [details]
Patch
Comment 3 mitz 2011-04-28 19:04:28 PDT
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?
Comment 4 Jon Lee 2011-04-28 19:06:17 PDT
(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.
Comment 5 Jon Lee 2011-04-28 19:21:03 PDT
Created attachment 91619 [details]
Patch
Comment 6 Darin Adler 2011-04-29 10:44:48 PDT
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 7 mitz 2011-04-29 10:49:02 PDT
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).
Comment 8 Jon Lee 2011-04-29 12:12:41 PDT
Created attachment 91717 [details]
Patch
Comment 9 mitz 2011-04-29 13:26:28 PDT
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!
Comment 10 Jon Lee 2011-04-29 13:43:36 PDT
Committed r85356: <http://trac.webkit.org/changeset/85356>