Bug 162008

Summary: dictionaryPopupInfoForRange() can change selection temporarily; updates should not be sent to the UIProcess.
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, sam, thorton, wenson_hsieh
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch thorton: review+

Beth Dakin
Reported 2016-09-14 23:39:24 PDT
dictionaryPopupInfoForRange() can change selection temporarily; updates should not be sent to the UIProcess. rdar://problem/28312297
Attachments
Patch (4.29 KB, patch)
2016-09-14 23:43 PDT, Beth Dakin 		no flags
DetailsFormatted DiffDiff
Patch (3.43 KB, patch)
2016-09-15 10:19 PDT, Beth Dakin 		thorton: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Beth Dakin
Comment 1 2016-09-14 23:43:16 PDT
Created attachment 288931 [details] Patch
Wenson Hsieh
Comment 2 2016-09-15 07:47:08 PDT
Comment on attachment 288931 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288931&action=review > Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:420 > + m_isGettingDictionaryPopupInfo = true; Can we use a TemporaryChange<bool> here?
Beth Dakin
Comment 3 2016-09-15 10:19:31 PDT
(In reply to comment #2) > Comment on attachment 288931 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288931&action=review > > > Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:420 > > + m_isGettingDictionaryPopupInfo = true; > > Can we use a TemporaryChange<bool> here? Oh, yes!
Beth Dakin
Comment 4 2016-09-15 10:19:50 PDT
Created attachment 288966 [details] Patch
Tim Horton
Comment 5 2016-09-15 10:52:15 PDT
Comment on attachment 288966 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288966&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4841 > + // Specifically, if there is a caret selection, it will change to a range selection of the word around the caret. And > + // then it will change back. Anything that makes a TextIndicator (among probably a few other things) will have this problem too. Find-in-page, force touch, etc.
Beth Dakin
Comment 6 2016-09-15 11:00:36 PDT
(In reply to comment #5) > Comment on attachment 288966 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288966&action=review > > > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4841 > > + // Specifically, if there is a caret selection, it will change to a range selection of the word around the caret. And > > + // then it will change back. > > Anything that makes a TextIndicator (among probably a few other things) will > have this problem too. Find-in-page, force touch, etc. This will cover many of those cases, but not find in page. https://trac.webkit.org/changeset/205983 Thanks Tim!
Note You need to log in before you can comment on or make changes to this bug.