Bug 216756

Summary: Selection API: A few more refinements to DOMSelection and VisibleSelection to pass all WPT tests
Product: WebKit Reporter: Darin Adler <darin>
Component: HTML EditingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ews-watchlist, mifenton, rniwa, sam, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 216325    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch rniwa: review+

Darin Adler
Reported 2020-09-20 17:37:25 PDT
Selection API: A few more refinements to DOMSelection and VisibleSelection to pass all WPT tests
Attachments
Patch (13.78 KB, patch)
2020-09-20 17:50 PDT, Darin Adler 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (14.26 KB, patch)
2020-09-20 17:56 PDT, Darin Adler 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (16.21 KB, patch)
2020-09-20 21:39 PDT, Darin Adler 		rniwa: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2020-09-20 17:50:36 PDT Comment hidden (obsolete)
Darin Adler
Comment 2 2020-09-20 17:56:12 PDT
Created attachment 409254 [details] Patch
Darin Adler
Comment 3 2020-09-20 21:09:10 PDT
Apparently changing how the selection direction is computed causes a change to this test result (editing/inserting/insert-list-in-table-cell-07.html). It’s not obviously making anything worse, but also not obviously better. I am thinking this is another case where re-basing is the right thing to do. The user-visible change is that the selection ends up inside the first table cell, where in the older case the selected ended up outside the table. Neither result seems right to me. I’d like to see the whole table end up selected and I don’t understand why that doesn’t happen.
Darin Adler
Comment 4 2020-09-20 21:39:41 PDT
Created attachment 409259 [details] Patch
Darin Adler
Comment 5 2020-09-21 11:51:19 PDT
All green on EWS, ready for review.
Darin Adler
Comment 6 2020-09-21 12:45:20 PDT
Comment on attachment 409259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409259&action=review > LayoutTests/ChangeLog:13 > + the current beahvior, before this patch and after, yields an insertion point. Misspelled behavior here.
Ryosuke Niwa
Comment 7 2020-09-21 13:01:03 PDT
Comment on attachment 409259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409259&action=review > Source/WebCore/editing/VisibleSelection.h:160 > + bool m_anchorIsFirst : 1; // True if the anchor is before the focus. > + bool m_isDirectional : 1; // On Mac, Shift-arrow keys move the anchor in a directional selection and moves either end to always extend in a non-directional selection. This is a bit tangential but it seems totally unnecessary to use bitfields here.
Darin Adler
Comment 8 2020-09-21 13:16:41 PDT
Comment on attachment 409259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409259&action=review >> Source/WebCore/editing/VisibleSelection.h:160 >> + bool m_isDirectional : 1; // On Mac, Shift-arrow keys move the anchor in a directional selection and moves either end to always extend in a non-directional selection. > > This is a bit tangential but it seems totally unnecessary to use bitfields here. I agree. I was just thinking about that last night, in fact!
Darin Adler
Comment 9 2020-09-21 13:23:31 PDT
Committed r267362: <https://trac.webkit.org/changeset/267362>
Radar WebKit Bug Importer
Comment 10 2020-09-21 13:24:16 PDT
<rdar://problem/69323482>
Note You need to log in before you can comment on or make changes to this bug.