Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix selection{Start,End} when selectionDirection is "backward" #9310

Merged

Conversation

Projects
None yet
3 participants
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 31, 2018

Per the spec, selectionStart and selectionEnd should return the same
values regardless of the selectionDirection. (That is, selectionStart is
always less than or equal to selectionEnd; the direction then implies
which of selectionStart or selectionEnd is the cursor position.)

There was no explicit WPT test for this, so I added one.

This bug was initially quite hard to wrap my head around, and I think
part of the problem is the code in TextInput. Therefore, in the process
of fixing it I have refactored the implementation of TextInput:

  • Rename selection_begin to selection_origin. This value doesn't
    necessarily correspond directly to the selectionStart DOM value - in
    the case of a backward selection, it corresponds to selectionEnd.
    I feel that "origin" doesn't imply a specific ordering as strongly as
    "begin" (or "start" for that matter) does.

  • In various other cases where "begin" is used as a synonym for "start",
    just use "start" for consistency.

  • Implement selection_start() and selection_end() methods (and their
    _offset() variants) which directly correspond to their DOM
    equivalents.

  • Rename other related methods to make them less wordy and more
    consistent / intention-revealing.

  • Add assertions to assert_ok_selection() to ensure that our assumptions
    about the ordering of selection_origin and edit_point are met. This
    then revealed a bug in adjust_selection_for_horizontal_change() where
    the value of selection_direction was not maintained correctly (causing
    a unit test failure when the new assertion failed).

Upstreamed from servo/servo#19544 [ci skip]


This change is Reviewable

Fix selection{Start,End} when selectionDirection is "backward"
Per the spec, selectionStart and selectionEnd should return the same
values regardless of the selectionDirection. (That is, selectionStart is
always less than or equal to selectionEnd; the direction then implies
which of selectionStart or selectionEnd is the cursor position.)

There was no explicit WPT test for this, so I added one.

This bug was initially quite hard to wrap my head around, and I think
part of the problem is the code in TextInput. Therefore, in the process
of fixing it I have refactored the implementation of TextInput:

* Rename selection_begin to selection_origin. This value doesn't
  necessarily correspond directly to the selectionStart DOM value - in
  the case of a backward selection, it corresponds to selectionEnd.
  I feel that "origin" doesn't imply a specific ordering as strongly as
  "begin" (or "start" for that matter) does.

* In various other cases where "begin" is used as a synonym for "start",
  just use "start" for consistency.

* Implement selection_start() and selection_end() methods (and their
  _offset() variants) which directly correspond to their DOM
  equivalents.

* Rename other related methods to make them less wordy and more
  consistent / intention-revealing.

* Add assertions to assert_ok_selection() to ensure that our assumptions
  about the ordering of selection_origin and edit_point are met. This
  then revealed a bug in adjust_selection_for_horizontal_change() where
  the value of selection_direction was not maintained correctly (causing
  a unit test failure when the new assertion failed).

Upstreamed from servo/servo#19544 [ci skip]
@servo-wpt-sync

This comment has been minimized.

Copy link
Collaborator Author

servo-wpt-sync commented Jan 31, 2018

Code reviewed upstream.

@servo-wpt-sync servo-wpt-sync merged commit 60202b0 into master Jan 31, 2018

@jdm jdm deleted the sync_cd3e94c6d773825794b78e792ed29d8bc57e16e3 branch Jan 31, 2018

@wpt-pr-bot wpt-pr-bot added the html label Jan 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.