Closed
Bug 180814
Opened 22 years ago
Closed 22 years ago
nsIControllers::InsertControllerAt() method should start numbering controller IDs with 1, not 0
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.3alpha
People
(Reporter: cmanske, Assigned: cmanske)
Details
Attachments
(1 file)
3.30 KB,
patch
|
Brade
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
nsIControllers::AppendController() was fixed recently so the ID assigned to each new controller should start with 1. InsertControllerAt() should do the same. This is so we can trust that "0" for a cached controller ID means we haven't created a controller yet.
Assignee | ||
Comment 1•22 years ago
|
||
This includes fixes to controller code in editor that relies on the controller ID.
Assignee | ||
Comment 2•22 years ago
|
||
Comment on attachment 106725 [details] [diff] [review] Fix v1 This fix was originally included in another bug and brade commented: I don't understand this block: + // Don't need to do this if already done + if (gComposerWindowControllerID) + return; Why isn't there a similar optimization for gComposerJSCommandControllerID? cmanske: Once we create a controller and get its ID, it must be non-zero so we can use the ID to test for creation. There is the same optimization for gComposerJSCommandControllerID, the line: + if (gComposerJSCommandControllerID) tests if that controller was already created and uses getControllerById instead of creating a new controller.
Attachment #106725 -
Flags: superreview?(sfraser)
Attachment #106725 -
Flags: review?(brade)
Assignee | ||
Updated•22 years ago
|
Comment 3•22 years ago
|
||
Comment on attachment 106725 [details] [diff] [review] Fix v1 + if (gComposerWindowControllerID) Would prefer + if (gComposerWindowControllerID > 0)
Attachment #106725 -
Flags: superreview?(sfraser) → superreview+
Comment 4•22 years ago
|
||
Comment on attachment 106725 [details] [diff] [review] Fix v1 probably this should be covered elsewhere, but I find it confusing to have such similarly named variables: gComposerJSCommandControllerID gComposerWindowControllerID Is there a comment that explains what each is for? r=brade
Attachment #106725 -
Flags: review?(brade) → review+
Assignee | ||
Comment 5•22 years ago
|
||
checked into 1.3a trunk. (I'll take care of controller ID comments elsewhere)
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [FIX IN HAND]
You need to log in
before you can comment on or make changes to this bug.
Description
•