Bug 150963 – Preview on apple.com/contact with all text selected shows a map

• Component: New Bugs
• Status: REOPENED

[Tim Horton]

Preview on apple.com/contact with all text selected shows a map

[Tim Horton]

Created attachment 264899 [details]
Patch

[Tim Horton]

http://trac.webkit.org/changeset/192089

[Ryan Haddad]

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'.

[Ryan Haddad]

Rolled out in <http://trac.webkit.org/changeset/192090>

[Tim Horton]

(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.

[Ahmad Saleem]

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!