Bug 201618 – Tap and hold on Facebook sometimes creates a tall empty selection

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Timothy Hatcher]

Most of Facebook has `-webkit-user-select: none`. The selection being created is spanning two of the only elements that are still user selectable — a user avatar icon in the story bar at the top, and the loading icon at the bottom.

[Timothy Hatcher]

<rdar://53630145>

[Timothy Hatcher]

<rdar://53630145>

[Timothy Hatcher]

Created attachment 378417 [details]
Patch

[Megan Gardner]

Comment on attachment 378417 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +

Do we know why this is no longer needed? That is my only qualm about this patch, I'm not following why we don't need this anymore. What changed to make it not needed.

[Alexey Proskuryakov]

Comment on attachment 378417 [details]
Patch

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

> Tools/ChangeLog:10
> +        * TestWebKitAPI/Tests/ios/SelectionByWord.mm: Added.

What necessitates was sing an API tests? Those are less scalable and have weaker infrastructure around them, so layout tests are preferred whenever possible.

[Timothy Hatcher]

Comment on attachment 378417 [details]
Patch

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

>> Source/WebCore/ChangeLog:16
>> +
> 
> Do we know why this is no longer needed? That is my only qualm about this patch, I'm not following why we don't need this anymore. What changed to make it not needed.

This code dates back to the original iOS upstream in 2014. The code was originally added as-is in 2013 when wordRangeFromPosition() was introduced, not as part of a bug fix. It was landed without a test, so I don't know anymore details on why it was originally needed.

VisualPosition and Position changes since then have likely made it not necessary and broken in cases like we see today on Facebook.

[Timothy Hatcher]

Comment on attachment 378417 [details]
Patch

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

>> Tools/ChangeLog:10
>> +        * TestWebKitAPI/Tests/ios/SelectionByWord.mm: Added.
> 
> What necessitates was sing an API tests? Those are less scalable and have weaker infrastructure around them, so layout tests are preferred whenever possible.

I wasn't able to get this to reproduce as needed in a LayoutTest -- I tried.

[Megan Gardner]

With the additional comments, r+

[WebKit Commit Bot]

Comment on attachment 378417 [details]
Patch

Clearing flags on attachment: 378417

Committed r249701: <https://trac.webkit.org/changeset/249701>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.