Bug 216756 - Selection API: A few more refinements to DOMSelection and VisibleSelection to pass all WPT tests
Summary: Selection API: A few more refinements to DOMSelection and VisibleSelection to...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks: 216325
  Show dependency treegraph
 
Reported: 2020-09-20 17:37 PDT by Darin Adler
Modified: 2020-09-21 13:24 PDT (History)
7 users (show)

See Also:


Attachments
Patch (13.78 KB, patch)
2020-09-20 17:50 PDT, Darin Adler
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.26 KB, patch)
2020-09-20 17:56 PDT, Darin Adler
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (16.21 KB, patch)
2020-09-20 21:39 PDT, Darin Adler
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2020-09-20 17:37:25 PDT
Selection API: A few more refinements to DOMSelection and VisibleSelection to pass all WPT tests
Comment 1 Darin Adler 2020-09-20 17:50:36 PDT Comment hidden (obsolete)
Comment 2 Darin Adler 2020-09-20 17:56:12 PDT
Created attachment 409254 [details]
Patch
Comment 3 Darin Adler 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.
Comment 4 Darin Adler 2020-09-20 21:39:41 PDT
Created attachment 409259 [details]
Patch
Comment 5 Darin Adler 2020-09-21 11:51:19 PDT
All green on EWS, ready for review.
Comment 6 Darin Adler 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.
Comment 7 Ryosuke Niwa 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.
Comment 8 Darin Adler 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!
Comment 9 Darin Adler 2020-09-21 13:23:31 PDT
Committed r267362: <https://trac.webkit.org/changeset/267362>
Comment 10 Radar WebKit Bug Importer 2020-09-21 13:24:16 PDT
<rdar://problem/69323482>