Chromium Code Reviews Chromium Code Reviews
Issue
2693983005:
Reland of Make FrameSelection to hold non-canonicalized positions (Closed)
DescriptionReland of Make FrameSelection to hold non-canonicalized positions (patchset #1 id:1 of https://codereview.chromium.org/2691243002/ )
Reason for revert:
testing the UI , ignore ...
Original issue's description:
> Revert of Make FrameSelection to hold non-canonicalized positions (patchset #9 id:180001 of https://codereview.chromium.org/2680943004/ )
>
> Reason for revert:
> See https://codereview.chromium.org/2680943004/#msg70
>
> Original issue's description:
> > Make FrameSelection to hold non-canonicalized DOM positions
> >
> > This patch makes |FrameSelection| to hold non-canonicalized DOM positions in
> > |SelectionEditor| to align Selection API specification[1] for improving
> > interoperatbility[2].
> >
> > Before this patch we holds selection as |VisibleSelection| as canonicalized
> > DOM positions. This behavior is not align with Selection API specification[1]
> > then the most complained issue of Blink from editing-tf@w3c.
> >
> > The heart of this patch is holding selection as |SelectionInDOMTree| and
> > compute |VisibleSelection| on-demand with cache of computed |VisibleSelection|.
> >
> > |VisibleSelection| cache is invalidate each DOM tree change and style change
> > since canonicalization referes CSS style properties, e.g display, visibility,
> > -webkit-user-modify, etc, and layout dimension.
> >
> > |SelectionEditor| utilizes |SynchronousMutationObserver| to relocate
> > |m_selectionInDOMTree| instead of |FrameSelection|. Before this patch
> > |FrameSelection| relocates |VisibleSelection| with dirty layout tree then
> > sets |FrameSelection::setSelection()|. To void cyclic reference between
> > |FrameSelection| and |SelectionEditor|, we could not move relocation part to
> > |SelectionEditor|.
> >
> > This patch also updates
> > |FrameSelection::updatePostionAfterAdoptingTextNodesMerged()| to handle
> > |PositonAnchorType|.
> >
> > # Highlight of changes
> > ## FrameCaret
> > - Compute caret position after "layout clean" rather than each selection change
> > to align rendering pipeline.
> >
> > ## CharacterData
> > Changes timing of notifying character data update for ease of relocation of
> > positions.
> >
> > ## FrameSelection
> > - Move |m_isHandleVisible| to |SelectionInDOMTree| as follow-up of [5].
> > - Move selection relocation to |SelectionEditor|; following patch will move
> > implementations to "SelectionEditor.cpp"
> >
> > ## SelecitonEdtior
> > - Make it to hold |SelectionInDOMTree| with relocation at DOM mutation.
> > - Caching |VisibleSelection|
> >
> > # Brief description of test expectation changes:
> > ## ImeTest.java:
> > This patch gets rid of redundant selection change event from
> > - |testImePaste|,
> > - |testContentEditableEvents_DeleteSurroundingText|
> > - |testInputTextEvents_DeleteSurroundingText|
> >
> > ## LayoutTests
> > Before this patch, Blink uses |VisibleSelection| when it sets even if style and
> > layout changed. This is wrong and unexpected behavior since positions in
> > |VisibleSelection| can no longer be canonicalized positions. This patch changes
> > this behavior to return "sane" canonicalized positions with clean style and
> > layout tree.
> >
> > This patch is the result of many attempts. Previous changes can be found in
> > [3][4].
> >
> > [1] https://www.w3.org/TR/selection-api/ W3C Selection API
> > [2] https://goo.gl/9v1zOK Improving Interoperatbility of Selection
> > [3] http://crrev.com/1958093002
> > [4] http://crrev.com/2637013002
> > [5] http://crrev.com/2651803007 Added isHandleVisible to |SelectionTemplate|
> >
> > BUG=139552, 603684, 605499, 606499, 625533, 644648, 679991
> > TEST=See changes in this patch
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> >
> > Review-Url: https://codereview.chromium.org/2680943004
> > Cr-Original-Commit-Position: refs/heads/master@{#449928}
> > Committed: https://chromium.googlesource.com/chromium/src/+/157413286770a7ac5a24c446a30c08f749738276
> > Review-Url: https://codereview.chromium.org/2680943004
> > Cr-Commit-Position: refs/heads/master@{#450280}
> > Committed: https://chromium.googlesource.com/chromium/src/+/17c84b2b6519c821dc319e79f2a7a4508508e20f
>
> TBR=changwan@chromium.org,tkent@chromium.org,xiaochengh@chromium.org,yoichio@chromium.org,yosin@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=139552, 603684, 605499, 606499, 625533, 644648, 679991
>
> Review-Url: https://codereview.chromium.org/2691243002
> Cr-Commit-Position: refs/heads/master@{#450291}
> Committed: https://chromium.googlesource.com/chromium/src/+/8f2a1baf12aa71d59d72e24583983b8a6b78340a
TBR=tkent@chromium.org,changwan@chromium.org,xiaochengh@chromium.org,yoichio@chromium.org,yosin@chromium.org,tyoshino@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=139552, 603684, 605499, 606499, 625533, 644648, 679991
Patch Set 1 #Messages
Total messages: 2 (1 generated)
|
