Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(31)

Issues Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 1202153004: Enable selection for the composed tree (Closed)

Created:
4 years, 10 months ago by hajimehoshi
Modified:
4 years, 10 months ago
Reviewers:
tkent, yosin_UTC9
CC:
blink-reviews, pdr+renderingwatchlist_chromium.org, zoltan1, szager+layoutwatch_chromium.org, eae+blinkwatch, leviw+renderwatch, blink-reviews-rendering, jchaffraix+rendering
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Enable selection for the composed tree This CL adds a new flag 'SelectionForComposedTree' and uses this. If the flag is on, the types for the composed tree like EditingInComposedTreeStrategy are passed to the template parameter instead of those of the DOM tree. Selection for the composed tree is implemented by switching tree- traversing strategies. Technically, this doesn't affect websites which don't use shadow DOMs. The problems would be on websites which use shadow DOMs if we have, and such websites account for only 2% now. In an emergency case, we can revert this by hiding the flag. This patch is result of collaboration work with yosin@chromium.org for selection of web components. We are now preparing this feature at crrev.com/1106433002. BUG=275851 TEST=./Tools/Scripts/run-webkit-tests editing/shadow/selection-shadow-mouse-drag.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197704

Patch Set 1 #

Total comments: 7

Patch Set 2 : tkent's review #

Patch Set 3 : Rename the flag #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -3 lines) Patch
A LayoutTests/editing/shadow/selection-shadow-mouse-drag.html View 1 chunk +19 lines, -0 lines 0 comments Download
A LayoutTests/editing/shadow/selection-shadow-mouse-drag-expected.html View 1 chunk +14 lines, -0 lines 0 comments Download
M Source/core/editing/FrameSelection.cpp View 1 2 7 chunks +26 lines, -2 lines 0 comments Download
M Source/core/editing/SelectionController.cpp View 1 2 3 chunks +8 lines, -0 lines 0 comments Download
M Source/core/editing/VisibleSelection.h View 6 chunks +17 lines, -0 lines 0 comments Download
M Source/core/editing/VisibleSelection.cpp View 6 chunks +33 lines, -1 line 0 comments Download
M Source/core/layout/LayoutView.cpp View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/layout/PendingSelection.cpp View 1 2 2 chunks +4 lines, -0 lines 0 comments Download
M Source/platform/RuntimeEnabledFeatures.in View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 19 (2 generated)
hajimehoshi
PTAL
4 years, 10 months ago (2015-06-23 10:16:51 UTC) #2
yosin_UTC9
In Patch Set #1: mac_blink_rel bot failure is inspector/elements/styles-3/styles-change-node-while-editing.html It doesn't relate to this patch.
4 years, 10 months ago (2015-06-23 13:32:43 UTC) #3
yosin_UTC9
https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/VisibleSelection.cpp File Source/core/editing/VisibleSelection.cpp (right): https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/VisibleSelection.cpp#newcode361 Source/core/editing/VisibleSelection.cpp:361: return expandUsingGranularity(granularity); We'll introduce composed tree version of |expandUsingGranularity()| ...
4 years, 10 months ago (2015-06-23 13:45:01 UTC) #4
yosin_UTC9
https://codereview.chromium.org/1202153004/diff/1/LayoutTests/editing/shadow/selection-shadow-mouse-drag.html File LayoutTests/editing/shadow/selection-shadow-mouse-drag.html (right): https://codereview.chromium.org/1202153004/diff/1/LayoutTests/editing/shadow/selection-shadow-mouse-drag.html#newcode1 LayoutTests/editing/shadow/selection-shadow-mouse-drag.html:1: <!DOCTYPE html> This test checks selection by mouse and ...
4 years, 10 months ago (2015-06-23 13:46:34 UTC) #5
tkent
https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/FrameSelection.cpp File Source/core/editing/FrameSelection.cpp (left): https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/FrameSelection.cpp#oldcode271 Source/core/editing/FrameSelection.cpp:271: if (m_selection == s) { Are there other instances ...
4 years, 10 months ago (2015-06-24 00:07:42 UTC) #6
tkent
> Enable selection for Web Components Do you mean "for Shadow DOM" or "for composed ...
4 years, 10 months ago (2015-06-24 00:08:51 UTC) #7
kochi
On 2015/06/24 00:07:42, tkent wrote: > https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/FrameSelection.cpp > File Source/core/editing/FrameSelection.cpp (left): > > https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/FrameSelection.cpp#oldcode271 > ...
4 years, 10 months ago (2015-06-24 03:25:23 UTC) #8
yosin_UTC9
On 2015/06/24 03:25:23, Takayoshi Kochi wrote: > On 2015/06/24 00:07:42, tkent wrote: > > > ...
4 years, 10 months ago (2015-06-24 03:42:11 UTC) #9
hajimehoshi
https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/FrameSelection.cpp File Source/core/editing/FrameSelection.cpp (left): https://codereview.chromium.org/1202153004/diff/1/Source/core/editing/FrameSelection.cpp#oldcode271 Source/core/editing/FrameSelection.cpp:271: if (m_selection == s) { On 2015/06/24 00:07:41, tkent ...
4 years, 10 months ago (2015-06-24 04:21:07 UTC) #10
hajimehoshi
> Do you mean "for Shadow DOM" or "for composed tree"? IMO both are almost ...
4 years, 10 months ago (2015-06-24 04:21:58 UTC) #11
hajimehoshi
Updated the description. PTAL
4 years, 10 months ago (2015-06-24 04:48:33 UTC) #12
tkent
> This CL adds a new flag 'SelectionForWebComponents' and uses this. If The flag name ...
4 years, 10 months ago (2015-06-24 04:51:23 UTC) #13
hajimehoshi
On 2015/06/24 04:51:23, tkent wrote: > > This CL adds a new flag 'SelectionForWebComponents' and ...
4 years, 10 months ago (2015-06-24 05:03:19 UTC) #14
tkent
lgtm
4 years, 10 months ago (2015-06-24 05:14:11 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1202153004/40001
4 years, 10 months ago (2015-06-24 05:15:09 UTC) #17
tkent
On 2015/06/24 05:14:11, tkent wrote: > lgtm Note that we discussed offline, and the conclusion ...
4 years, 10 months ago (2015-06-24 05:16:04 UTC) #18
commit-bot: I haz the power
4 years, 10 months ago (2015-06-24 06:09:53 UTC) #19
Message was sent while issue was closed. 
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197704

Powered by Google App Engine
This is Rietveld 408576698