| Summary: | Unable to select multiple lines of vertical text correctly | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Megan Gardner <megan_gardner> | ||||||||||
| Component: | New Bugs | Assignee: | Megan Gardner <megan_gardner> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | darin, wenson_hsieh | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Megan Gardner
2020-06-26 18:06:56 PDT
Created attachment 402937 [details]
Patch
Comment on attachment 402937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402937&action=review It would also be nice to add a new layout test for this, as well. > Source/WebKit/ChangeLog:3 > + Unable to select multiple lines of vertial text correclty. Nits - vertial => vertical correclty => correctly > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1501 > + if (existingSelection.rootEditableElement()->computedStyle()->isVerticalWritingMode()) { Wouldn't this only work when the selection is in editable content? I think this will also crash when trying to move selection handles in non-editable text on iOS :P We probably want to check the RenderStyle of the container node of the base VisiblePosition instead. Created attachment 405741 [details]
Patch
Comment on attachment 405741 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405741&action=review > Source/WebKit/ChangeLog:14 > + In order to make for a better text selection experience, we pulled the selection position > + down to be on the last line selectable, rather than snap the selection to a single position. > + This made for a better selection experience on small text, but we failed to take > + vertical text into account, and a user is locked into only selecting vertical text that ends below the > + other anchor point of the selection. We should have the same behavior for vertical text, but correctly > + calculated for X instead of Y. Can we create a regression test? > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1497 > + WebCore::Node *node = selectionStart.deepEquivalent().containerNode(); auto node = or WebCore::Node* node = Comment on attachment 405741 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405741&action=review >> Source/WebKit/ChangeLog:14 >> + calculated for X instead of Y. > > Can we create a regression test? The selection test infrastructure is currently broken by changes in UIKit. There are plans to change how those tests are written, and I will add a test with those changes. Created attachment 405748 [details]
Patch
Created attachment 405772 [details]
Patch for landing
Committed r265174: <https://trac.webkit.org/changeset/265174> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405772 [details]. |