Bug 150963 - Preview on apple.com/contact with all text selected shows a map
Summary: Preview on apple.com/contact with all text selected shows a map
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on: 150966
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-05 16:27 PST by Tim Horton
Modified: 2022-10-05 15:35 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.75 KB, patch)
2015-11-05 16:27 PST, Tim Horton
bdakin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2015-11-05 16:27:18 PST
Preview on apple.com/contact with all text selected shows a map
Comment 1 Tim Horton 2015-11-05 16:27:49 PST
Created attachment 264899 [details]
Patch
Comment 2 Tim Horton 2015-11-05 16:39:55 PST
http://trac.webkit.org/changeset/192089
Comment 3 Ryan Haddad 2015-11-05 20:18:45 PST
This change appears to have broken layout test editing/mac/dictionary-lookup/dictionary-lookup-inside-selection.html

Run:
<https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Tests%29/builds/9696>
Results:
<https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK2%20(Tests)/r192089%20(9696)/results.html>

--- /Volumes/Data/slave/yosemite-release-tests-wk2/build/layout-test-results/editing/mac/dictionary-lookup/dictionary-lookup-inside-selection-expected.txt
+++ /Volumes/Data/slave/yosemite-release-tests-wk2/build/layout-test-results/editing/mac/dictionary-lookup/dictionary-lookup-inside-selection-actual.txt
@@ -1,3 +1,3 @@
 Some text to not look up. This part is all selected, and should be taken as a unit when looked up.
 
-Lookup string for inside selected text: 'This part is all selected, and should be taken as a unit when looked up.'.
+Lookup string for inside selected text: 'unit'.
Comment 4 Ryan Haddad 2015-11-05 20:33:01 PST
Rolled out in <http://trac.webkit.org/changeset/192090>
Comment 5 Tim Horton 2015-11-10 14:39:24 PST
(In reply to comment #3)
> This change appears to have broken layout test
> editing/mac/dictionary-lookup/dictionary-lookup-inside-selection.html
> 
> Run:
> <https://build.webkit.org/builders/
> Apple%20Yosemite%20Release%20WK2%20%28Tests%29/builds/9696>
> Results:
> <https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK2%20(Tests)/
> r192089%20(9696)/results.html>
> 
> ---
> /Volumes/Data/slave/yosemite-release-tests-wk2/build/layout-test-results/
> editing/mac/dictionary-lookup/dictionary-lookup-inside-selection-expected.txt
> +++
> /Volumes/Data/slave/yosemite-release-tests-wk2/build/layout-test-results/
> editing/mac/dictionary-lookup/dictionary-lookup-inside-selection-actual.txt
> @@ -1,3 +1,3 @@
>  Some text to not look up. This part is all selected, and should be taken as
> a unit when looked up.
>  
> -Lookup string for inside selected text: 'This part is all selected, and
> should be taken as a unit when looked up.'.
> +Lookup string for inside selected text: 'unit'.

Hmm, I can't reproduce this on El Capitan.
Comment 6 Ahmad Saleem 2022-10-05 15:34:53 PDT
It seems this patch was rollout and never landed again.

We don't have --> class VisiblePosition below:

https://github.com/WebKit/WebKit/blob/8c52a9581938140ed6bd78e05e8e60898038b383/Source/WebCore/editing/mac/DictionaryLookup.h#L52

Don't have "VisiblePosition":

https://github.com/WebKit/WebKit/blob/89eea21b4ef219726324da09a77312748f7688f0/Source/WebCore/editing/cocoa/DictionaryLookup.mm#L264

and missing bits here:

https://github.com/WebKit/WebKit/blob/89eea21b4ef219726324da09a77312748f7688f0/Source/WebCore/editing/cocoa/DictionaryLookup.mm#L272

____

In Summary, I don't think patch has been applied in any shape or form. Plus, it didn't had any test to verify whether it issue reproduces in current Safari build or not.

Appreciate if someone can have look and see if this is needed or not and rebase if needed. Thanks!