Summary: | Restrict Selection in contenteditable to the extent of that contenteditable | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Megan Gardner <megan_gardner> | ||||||||||||
Component: | New Bugs | Assignee: | Megan Gardner <megan_gardner> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bdakin, commit-queue, ews-watchlist, rniwa, simon.fraser, thorton, wenson_hsieh | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Megan Gardner
2018-06-18 17:40:42 PDT
Created attachment 342996 [details]
Patch
Comment on attachment 342996 [details] Patch Attachment 342996 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8244791 New failing tests: http/tests/security/canvas-remote-read-remote-video-localhost.html http/tests/preload/onload_event.html Created attachment 343038 [details]
Archive of layout-test-results from ews206 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 343065 [details]
Patch
Comment on attachment 343065 [details] Patch Attachment 343065 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/8249991 New failing tests: accessibility/mac/selection-notification-focus-change.html Created attachment 343071 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 343065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343065&action=review This looks reasonable to me. Can we write a test for this? > Source/WebKit/ChangeLog:9 > + a single content editable on iOS. There is functionality to esure this on mac, so it has been Nit - esure > Source/WebKit/ChangeLog:10 > + exposed and utalized for restricting the extent of a selection. Nit - utalized > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1339 > + VisiblePosition targetPosition = m_page->mainFrame().eventHandler().selectionExtentRespectingEditingBoundary(frame.selection().selection(), result.localPoint(), result.targetNode()); Nit - I don't think this local variable adds much. Also, I don't think we're guaranteed to find a target node when hit-testing, yet selectionExtentRespectingEditingBoundary requires a non-null targetNode. We should probably just make selectionExtentRespectingEditingBoundary take a Node& and add a null check here. Comment on attachment 343065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343065&action=review As I said in the WebCore log, my system currently won't allow me to run any tests, but I will write one as soon as I can update. >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1339 >> + VisiblePosition targetPosition = m_page->mainFrame().eventHandler().selectionExtentRespectingEditingBoundary(frame.selection().selection(), result.localPoint(), result.targetNode()); > > Nit - I don't think this local variable adds much. > > Also, I don't think we're guaranteed to find a target node when hit-testing, yet selectionExtentRespectingEditingBoundary requires a non-null targetNode. We should probably just make selectionExtentRespectingEditingBoundary take a Node& and add a null check here. There's too much in selectionExtentRespectingEditingBoundary that relies on taking a Node*, but I will add null checks both here and in the function. Created attachment 343107 [details]
Patch
Comment on attachment 343107 [details] Patch Clearing flags on attachment: 343107 Committed r233016: <https://trac.webkit.org/changeset/233016> All reviewed patches have been landed. Closing bug. |