Bug 213671 – Unable to select multiple lines of vertical text correctly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Unable to select multiple lines of vertial text correclty.

[Megan Gardner]

Created attachment 402937 [details]
Patch

[Megan Gardner]

<rdar://problem/53753636>

[Wenson Hsieh]

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.

[Megan Gardner]

Created attachment 405741 [details]
Patch

[Darin Adler]

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 =

[Megan Gardner]

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.

[Megan Gardner]

Created attachment 405748 [details]
Patch

[Megan Gardner]

Created attachment 405772 [details]
Patch for landing

[EWS]

Committed r265174: <https://trac.webkit.org/changeset/265174>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405772 [details].