Closed
Bug 857542
Opened 11 years ago
Closed 11 years ago
CustomizableUI.registerWindow() isn't always called
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: Unfocused, Assigned: mconley)
References
Details
Attachments
(1 file, 1 obsolete file)
3.85 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
Seems this was added in the initial review, and I missed it... CustomizeMode is calling CustomizableUI.registerWindow(), therefore it only gets called on windows that enter the customization mode. Instead, it needs to be called for every window that has a toolbar/panel that gets built by CustomizableUI. registerWindow() really just exists to ensure unregisterBuildWindow() gets called when the window closes - that cleans up the internal state of CustomizableUI that keeps track of build areas and widget instances. This was why the original code was hooking that up automatically whenever a build area (toolbar/panel) instance was registered - its cleaning up after those disappear (it's not so much about the window, but whats in the window). I think the intent was to decouple that, but I'm not convinced it should be decoupled.
Assignee | ||
Comment 1•11 years ago
|
||
Ah, yep - I see now. This patch moves the cleanup event handler registration back to when we register the toolbar.
Reporter | ||
Comment 2•11 years ago
|
||
Comment on attachment 735340 [details] [diff] [review] Patch v1 Review of attachment 735340 [details] [diff] [review]: ----------------------------------------------------------------- Should also: * remove the public API for registerWindow (don't think it will cause harm by someone calling it, but it's still not an API that should be called by outside code) * rename registerWindow to registerBuildWindow, so it matches unregisterBuildWindow * move registerBuildWindow so it's right before unregisterBuildWindow r+ with those tweaks.
Attachment #735340 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Thanks Blair!
Attachment #735340 -
Attachment is obsolete: true
Attachment #735507 -
Flags: review+
Assignee | ||
Comment 4•11 years ago
|
||
Landed on jamun: https://hg.mozilla.org/projects/jamun/rev/dc0040f952a5
Whiteboard: [fixed in jamun]
Assignee | ||
Comment 5•11 years ago
|
||
Landed on UX: https://hg.mozilla.org/projects/ux/rev/dc0040f952a5
Whiteboard: [fixed in jamun] → [fixed in jamun][fixed in ux]
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca22fecd3a71
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in jamun][fixed in ux]
Target Milestone: --- → Firefox 28
You need to log in
before you can comment on or make changes to this bug.
Description
•