Clean up action enums

RESOLVED FIXED in mozilla15

Status

()

Core
Editor
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ayg, Assigned: ayg)

Tracking

Trunk
mozilla15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We currently have at least three different enums that correspond to operation ID's:

nsEditor::OperationID:
  enum OperationID
  {
    kOpIgnore = -1,
    kOpNone = 0,
    kOpUndo,
    kOpRedo,
    kOpInsertNode,
    kOpCreateNode,
    kOpDeleteNode,
    kOpSplitNode,
    kOpJoinNode,
    kOpDeleteSelection,
    // text commands
    kOpInsertBreak    = 1000,
    kOpInsertText     = 1001,
    kOpInsertIMEText  = 1002,
    kOpDeleteText     = 1003
  };
<http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditor.h#121>

nsHTMLEditor::OperationID:
  enum OperationID
  {
    kOpInsertBreak         = 3000,
    kOpMakeList            = 3001,
    kOpIndent              = 3002,
    kOpOutdent             = 3003,
    kOpAlign               = 3004,
    kOpMakeBasicBlock      = 3005,
    kOpRemoveList          = 3006,
    kOpMakeDefListItem     = 3007,
    kOpInsertElement       = 3008,
    kOpInsertQuotation     = 3009,
    kOpSetTextProperty     = 3010,
    kOpRemoveTextProperty  = 3011,
    kOpHTMLPaste           = 3012,
    kOpLoadHTML            = 3013,
    kOpResetTextProperties = 3014,
    kOpSetAbsolutePosition = 3015,
    kOpRemoveAbsolutePosition = 3016,
    kOpDecreaseZIndex      = 3017,
    kOpIncreaseZIndex      = 3018
  };
<http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/nsHTMLEditor.h#112>

nsTextEditRules anonymous enum:
  enum 
  {
    kDefault             = 0,
    // any editor that has a txn mgr
    kUndo                = 1000,
    kRedo                = 1001,
    // text actions
    kInsertText          = 2000,
    kInsertTextIME       = 2001,
    kDeleteSelection     = 2002,
    kSetTextProperty     = 2003,
    kRemoveTextProperty  = 2004,
    kOutputText          = 2005,
    // html only action
    kInsertBreak         = 3000,
    kMakeList            = 3001,
    kIndent              = 3002,
    kOutdent             = 3003,
    kAlign               = 3004,
    kMakeBasicBlock      = 3005,
    kRemoveList          = 3006,
    kMakeDefListItem     = 3007,
    kInsertElement       = 3008,
    kLoadHTML            = 3013,
    kSetAbsolutePosition = 3015,
    kRemoveAbsolutePosition = 3016,
    kDecreaseZIndex      = 3017,
    kIncreaseZIndex      = 3018
  };
<http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/text/nsTextEditRules.h#80>

Notice that 1000 is both kUndo and kOpInsertBreak, and 1001 is both kRedo and kOpInsertText.  Note also that nsEditor::kOpInsertBreak is 1000, while nsHTMLEditor::kOpInsertBreak is 3000.  And a whole bunch of the anonymous nsTextEditRules actions overlap with the ns(HTML)Editor ones under different numbers.

Of course, everything that handles these actions accepts a PRInt32 as input, so there's no type safety at all.  Making this into one big enum would be a lot saner, and possibly fix some lurking bugs -- particularly if kUndo/kOpInsertBreak or kRedo/kOpInsertText are being conflated somewhere.
Created attachment 620684 [details] [diff] [review]
Patch v1

This started as a cleanup patch for bug 748303, but was big enough that I thought it deserved its own bug.  Since 1000 and 1001 were previously ambiguous, I made sure that no member had those numbers anymore.  In cases where the same action had different numbers, like kOpInsertBreak, or kInsertText vs. kOpInsertText, I just picked one.

I looked briefly at comm-central to see if it used any of these methods, and didn't see any.  I didn't do anything exciting like actually trying to compile it.

Try: https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Attachment #620684 - Flags: review?(ehsan)

Comment 2

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 170
    warnings: 33
    failure: 1
    other: 17
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 3

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 172
    warnings: 33
    failure: 3
    other: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 4

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 174
    warnings: 33
    failure: 3
    other: 11
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 5

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 175
    warnings: 33
    failure: 3
    other: 10
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 6

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 179
    warnings: 33
    failure: 3
    other: 6
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 7

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 180
    warnings: 34
    failure: 3
    other: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 8

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 182
    warnings: 34
    failure: 3
    other: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.

Comment 9

5 years ago
Try run for 3130eb84de9c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3130eb84de9c
Results (out of 221 total builds):
    success: 183
    warnings: 34
    failure: 3
    other: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-3130eb84de9c
 Timed out after 12 hours without completing.
Attachment #620684 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/94ce5f33a9ea
Flags: in-testsuite-
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/94ce5f33a9ea
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.