Bug 201618

Summary: Tap and hold on Facebook sometimes creates a tall empty selection
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: WebKit Misc.Assignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, enrica, megan_gardner, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Timothy Hatcher
Reported 2019-09-09 16:36:58 PDT
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.
Attachments
Patch (10.50 KB, patch)
2019-09-09 16:45 PDT, Timothy Hatcher 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Timothy Hatcher
Comment 1 2019-09-09 16:37:13 PDT Comment hidden (obsolete)
Timothy Hatcher
Comment 2 2019-09-09 16:37:13 PDT
<rdar://53630145>
Timothy Hatcher
Comment 3 2019-09-09 16:45:47 PDT
Created attachment 378417 [details] Patch
Megan Gardner
Comment 4 2019-09-09 17:26:08 PDT
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 5 2019-09-09 18:15:25 PDT
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 6 2019-09-09 19:13:54 PDT
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 7 2019-09-09 19:14:34 PDT
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
Comment 8 2019-09-09 19:46:12 PDT
With the additional comments, r+
WebKit Commit Bot
Comment 9 2019-09-09 20:50:10 PDT
Comment on attachment 378417 [details] Patch Clearing flags on attachment: 378417 Committed r249701: <https://trac.webkit.org/changeset/249701>
WebKit Commit Bot
Comment 10 2019-09-09 20:50:12 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.