New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 596064 link

Starred by 1 user
Status: Fixed
Owner:
█ chongz@chromium.org
Last visit > 30 days ago
Closed: Apr 2016
Cc:
dtapu...@chromium.org
yosin@chromium.org
esprehn@chromium.org
ojan@chromium.org
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 251275



Sign in to add a comment

Use enum instead of string command name in EditorCommand::CommandMap

Project Member Reported by chongz@chromium.org, Mar 18 2016 
Currently EditorCommand::CommandMap is using string to map command, which is error prone(see issue 251275). Instead we should replace them with an enum (probably in ../public/web), and have a map between enum and command name.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 12 2016 

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

commit 2dcc34c1e14d6847a6340da3d03fbe5e04801bfb
Author: chongz <chongz@chromium.org>
Date: Tue Apr 12 16:05:17 2016

Add enum class |WebEditingCommandType| for EditorCommand

Motivation: Using strings for actions is error prone (see
http://crbug.com/251275) and ideally we want to replace
string names with enum (e.g. replace strings in
https://goo.gl/VeUZFX). Also with enum we could eliminate
the huge |EditorCommand::CommandMap| with switch statements
to improve readability.

This CL is the first step and adds |WebEditingCommandType| enum class in public/platform/ and
provides conversion between string command name and enum type in
EditorCommand.cpp.

Also |EditorInternalCommand.idForUserMetrics| is replaced by
|EditorInternalCommand.commandType| but remains the same enum value, so
the user matrics won't be affected.

BUG= 596064 

Review URL: https://codereview.chromium.org/1841143002

Cr-Commit-Position: refs/heads/master@{#386702}

[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/core/core.gypi
[add] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/core/editing/EditingCommandTest.cpp
[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
[add] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/core/editing/commands/EditorCommandNames.h
[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/wtf/text/StringImpl.cpp
[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/wtf/text/StringImpl.h
[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/wtf/text/WTFString.cpp
[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/Source/wtf/text/WTFString.h
[modify] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/public/blink_headers.gypi
[add] https://crrev.com/2dcc34c1e14d6847a6340da3d03fbe5e04801bfb/third_party/WebKit/public/platform/WebEditingCommandType.h

Comment 2 by chongz@chromium.org, Apr 12 2016

Labels: M-52
Status: Fixed (was: Started)
►

Sign in to add a comment