Bug 187556 – Keep Selections within Shadow DOM boundaries

• Component: New Bugs
• Status: RESOLVED FIXED

[Megan Gardner]

Keep Selections within Shadow Dom boundries

[Megan Gardner]

Created attachment 344771 [details]
Patch

[Tim Horton]

Comment on attachment 344771 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Keep Selections within Shadow Dom boundries

DOM is usually 'DOM'

> Source/WebCore/editing/VisibleSelection.h:110
> +    WEBCORE_EXPORT Position adjustPositionForEnd(const Position& currentPosition, Node* startContainerNode) const;
> +    WEBCORE_EXPORT Position adjustPositionForStart(const Position& currentPosition, Node* startContainerNode) const;

IF it's right to expose these (no idea), I think they should be static (and thus not const).

[Megan Gardner]

Created attachment 344773 [details]
Patch

[EWS Watchlist]

Comment on attachment 344773 [details]
Patch

Attachment 344773 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8506962

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html

[EWS Watchlist]

Created attachment 344781 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Megan Gardner]

<rdar://problem/41664567>

[Ryosuke Niwa]

Comment on attachment 344773 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1221
> +    VisiblePosition position = frame->visiblePositionForPoint(pointInDocument);

Why do we need this? Can't we simply call deepEquivalent() on it directly?

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1231
> +    
> +    
> +    

Remove redundant blank lines?

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1232
> +    Position selectionStart = frame->selection().selection().visibleStart().deepEquivalent();

Seems like we should store frame->selection().selection() in a local temporary variable.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1236
> +        bool wouldFlip = comparePositions(result, selectionStart) <= 0;

Instead of wouldFlip. We should probably call it resultIsBeforeStart

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1237
> +        

Nit: Whitespace.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1240
> +        

Ditto.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1243
> +        

Ditto.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1247
> +        bool wouldFlip = comparePositions(selectionEnd, result) <= 0;

Ditto.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1248
> +        

Nit: Whitespace.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1254
> +        

Nit: Whitespace.

[Megan Gardner]

Created attachment 344867 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 344867 [details]
Patch for landing

Clearing flags on attachment: 344867

Committed r233778: <https://trac.webkit.org/changeset/233778>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.