Last Comment Bug 751547 - Clean up action enums
: Clean up action enums
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla15
Assigned To: Aryeh Gregor (:ayg) (next working March 28-April 26)
:
: Makoto Kato [:m_kato]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 06:13 PDT by Aryeh Gregor (:ayg) (next working March 28-April 26)
Modified: 2012-05-05 20:35 PDT (History)
2 users (show)
ayg: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (80.62 KB, patch)
2012-05-03 06:42 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
ehsan: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-05-03 06:13:03 PDT
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.
Comment 1 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-05-03 06:42:38 PDT
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
Comment 2 Mozilla RelEng Bot 2012-05-03 19:00:23 PDT
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 Mozilla RelEng Bot 2012-05-03 19:30:23 PDT
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 Mozilla RelEng Bot 2012-05-03 19:45:22 PDT
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 Mozilla RelEng Bot 2012-05-03 20:00:24 PDT
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 Mozilla RelEng Bot 2012-05-03 20:15:21 PDT
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 Mozilla RelEng Bot 2012-05-03 20:30:23 PDT
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 Mozilla RelEng Bot 2012-05-03 20:45:22 PDT
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 Mozilla RelEng Bot 2012-05-03 21:30:21 PDT
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.
Comment 10 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-05-05 11:53:33 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/94ce5f33a9ea
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-05-05 20:35:54 PDT
https://hg.mozilla.org/mozilla-central/rev/94ce5f33a9ea

Note You need to log in before you can comment on or make changes to this bug.