New issue
Advanced search Search tips

Issue 761254 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 706527



Sign in to add a comment

Reorganize core/editing/

Project Member Reported by yosin@chromium.org, Sep 1 2017

Issue description

As of today we have 132 files in core/editing/. It is too many for one
directory.

We would like to have:

ime/
 - ImeTextSpan.{cpp.h}
 - ImeTextSpanVectorBuilder.{cpp,h}
 - InputMethodController.{cpp,h}
 - InputMethodControllerTest.cpp

finder/
 - FindingInPageCoordinates.{cpp,h}
 - FindOptions.h
 - TextFinder.{cpp,h}
 - TextFinderTest.cpp

testing/
 - EditingTestBase.{cpp,h}


Files used only testing should be consolidated into testing/ directory.
 
Owner: shanmug...@samsung.com
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1

commit 2b7d4df9a3611ff420bb4bf0a5ad935008c601f1
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Fri Sep 15 05:47:18 2017

Reorg core/editing directory

Move files which are used only testing to testing/
directory.

Bug: 761254
Change-Id: I731cce4f237faf000076233d541afc08987b8f87
Reviewed-on: https://chromium-review.googlesource.com/666877
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502168}
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/dom/ElementTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/dom/NodeTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/dom/RangeTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/dom/TextTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/EditingCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/EditingStrategyTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/EditingStyleTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/EditingUtilitiesTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/EditorTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/FrameCaretTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/LayoutSelectionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/PositionIteratorTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/PositionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/RelocatablePositionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/SelectionControllerTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/SelectionModifierTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/SelectionTemplateTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/VisiblePositionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/VisibleUnitsTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/VisibleUnitsWordTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/CompositeEditCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/InsertListCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/InsertTextCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/SetCharacterDataCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/SplitTextNodeCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/commands/TypingCommandTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/iterators/BackwardsTextBufferTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/iterators/CharacterIteratorTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/iterators/SearchBufferTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/iterators/SimplifiedBackwardsTextIteratorTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImplTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImplTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImplTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/serializers/StyledMarkupSerializerTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.h
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionControllerTest.cpp
[rename] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/testing/EditingTestBase.cpp
[rename] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/testing/EditingTestBase.h
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/editing/testing/SelectionSampleTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/2b7d4df9a3611ff420bb4bf0a5ad935008c601f1/third_party/WebKit/Source/core/svg/SVGTextContentElementTest.cpp

Blocking: 706527
Not sure which one of the two issues block the other, but they are definitely related.

Comment 4 Deleted

I have some other ideas about the reorganization. Instead of having divisions of position and selection, how about introducing a model/ directory:

model/
 - EphemeralRange.{cpp,h}
 - PlainTextRange.{cpp,h}
 - Position.{cpp,h}
 - PositionTest.cpp
 - SelectionTemplate.{cpp,h}
 - SelectionTemplateTet.cpp
 - SelectionType.h
 - TextAffinity.{cpp,h}
 - VisiblePosition.{cpp,h}
 - VisiblePositionTest.cpp
 - VisibleSelection.{cpp,h}
 - VisibleSelectionTest.cpp

Then selection/ direction contains support only for frame selection. And there shouldn't be a position/ directory.

Notes:
1. classes in model/ are widely used in all editing and editing-related component, so they should be grouped together
2. Although PlainTextRange is extensively used by IME, it's a general class and I suggested AX using it.
3. RelocatablePosition is only used by commands and should go to commands/
4. PositionIterator is only used by canonicalization, and should be grouped together with EditingUtilities and VisibleUnits
5. RenderedPosition is a myth nobody understands (sigh...) and shouldn't be moved for now
We (tkent@, xiaochengh@, yoichio@, yosin@) had an offline discussion but didn't find a good idea to reorganize the classes other than ime and finder. The conclusion is to leave the files at their original places other than:

ime/
 - ImeTextSpan.{cpp.h}
 - ImeTextSpanVectorBuilder.{cpp,h}
 - InputMethodController.{cpp,h}
 - InputMethodControllerTest.cpp

finder/
 - FindingInPageCoordinates.{cpp,h}
 - FindOptions.h
 - TextFinder.{cpp,h}
 - TextFinderTest.cpp
Description: Show this description
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3e6b4ed329668a56f6d97c126fc637054a39d2a

commit a3e6b4ed329668a56f6d97c126fc637054a39d2a
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Wed Oct 11 08:03:44 2017

Reorg core/editing directory

Move files which are related to ime to ime/ directory.

Bug: 761254
Change-Id: I8e42b3d62d9836ac179d1adc3a736d2b8621b35c
Reviewed-on: https://chromium-review.googlesource.com/706752
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507922}
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/Editor.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ImeTextSpanTest.cpp
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/ImeTextSpan.cpp
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/ImeTextSpan.h
[add] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/ImeTextSpanTest.cpp
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/ImeTextSpanVectorBuilder.cpp
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/ImeTextSpanVectorBuilder.h
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/InputMethodController.cpp
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/InputMethodController.h
[rename] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/editing/ime/InputMethodControllerTest.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/events/KeyboardEvent.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/exported/WebInputMethodControllerImpl.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/frame/WebFrameWidgetImpl.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/a3e6b4ed329668a56f6d97c126fc637054a39d2a/third_party/WebKit/Source/core/page/FocusController.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b4ca8748d57cf30daff14721ef23eb636196989

commit 2b4ca8748d57cf30daff14721ef23eb636196989
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Tue Oct 24 02:42:06 2017

Reorg core/editing directory

Move files which are related to find to finder/ directory.

Bug: 761254
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_dbg_ng
Change-Id: I94a7f1fbaacd37e3f07eca87a4b1c82130f326b4
Reviewed-on: https://chromium-review.googlesource.com/720175
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511014}
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/Editor.h
[rename] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/finder/FindInPageCoordinates.cpp
[rename] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/finder/FindInPageCoordinates.h
[rename] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/finder/FindOptions.h
[rename] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/finder/TextFinder.cpp
[rename] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/finder/TextFinder.h
[rename] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/finder/TextFinderTest.cpp
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/iterators/SearchBuffer.h
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/editing/iterators/TextIterator.h
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/2b4ca8748d57cf30daff14721ef23eb636196989/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18a29730ead5f3f16fa1bbea71cc1a5dd4da4ecf

commit 18a29730ead5f3f16fa1bbea71cc1a5dd4da4ecf
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Nov 13 02:42:37 2017

Remove leftover core/editing/ImeTextSpanTest.cpp

File core/editing/ImeTextSpanTest.cpp is a leftover after we moved IME-
related files to core/editing/ime. It is no longer listed in BUILD.gn,
and is identical to core/editing/ime/ImeTextSpanTest.cpp.

Hence this patch removes the redundant file.

Bug: 761254
Change-Id: If4227e675b2866b34ac3f505f41a98e2da06ba26
Reviewed-on: https://chromium-review.googlesource.com/764876
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515877}
[delete] https://crrev.com/d391eaeb40118cead3f9e6666503b1d865cbfb2e/third_party/WebKit/Source/core/editing/ImeTextSpanTest.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebc1d8cf13d3c538a2e55be8c7aed91466afa5bc

commit ebc1d8cf13d3c538a2e55be8c7aed91466afa5bc
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Nov 20 07:20:09 2017

Move EditingCommandTest.cpp to editing/commands

Bug: 761254
Change-Id: I6d85d36bd47b845da8e904f2c2f0999f8b958a70
Reviewed-on: https://chromium-review.googlesource.com/777591
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517771}
[modify] https://crrev.com/ebc1d8cf13d3c538a2e55be8c7aed91466afa5bc/third_party/WebKit/Source/core/editing/BUILD.gn
[rename] https://crrev.com/ebc1d8cf13d3c538a2e55be8c7aed91466afa5bc/third_party/WebKit/Source/core/editing/commands/EditingCommandTest.cpp

Is there any thing pending for this issue ?
#12: Nope, before anyone gets a new idea.
Owner: ----
Status: Available (was: Assigned)
Project Member

Comment 15 by sheriffbot@chromium.org, Jan 8

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)

Sign in to add a comment