Closed
Bug 763932
Opened 12 years ago
Closed 12 years ago
[devtb] Add the right buttons to the Developer Toolbar
Categories
(DevTools :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 16
People
(Reporter: paul, Assigned: paul)
References
Details
(Whiteboard: [fixed-in-fx-team])
Attachments
(1 file, 7 obsolete files)
20.63 KB,
patch
|
Details | Diff | Splinter Review |
gcli / Console / Inspector / Style Editor / Debugger / DevTools menubutton
Updated•12 years ago
|
Summary: Add the right buttons to the Developer Toolbar → [devtb] Add the right buttons to the Developer Toolbar
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → paul
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #633599 -
Flags: feedback?(cedricv)
Assignee | ||
Comment 3•12 years ago
|
||
To integrate the style editor, I needed to implement a StyleEditor Manager that keeps track of opened editor. The button is checked only of the page is associated with a StyleEditor instance.
Assignee | ||
Comment 4•12 years ago
|
||
I won't work on the devtools menu in this bug. See bug 765564.
No longer depends on: 754481
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #633599 -
Attachment is obsolete: true
Attachment #633599 -
Flags: feedback?(cedricv)
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #633598 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #633885 -
Flags: review?(jwalker)
Assignee | ||
Updated•12 years ago
|
Attachment #633886 -
Flags: review?(cedricv)
Comment 7•12 years ago
|
||
Comment on attachment 633885 [details] [diff] [review] style editor - v1 Review of attachment 633885 [details] [diff] [review]: ----------------------------------------------------------------- Please could you take a look at GcliCommands.jsm:273 - I suspect the implementation of the 'edit foo' might do with a tweak. I have looked at StyleEditorManager and it looks OK to me, but I have made the assumption that Cedricv will be taking a deeper look.
Attachment #633885 -
Flags: review?(jwalker) → review+
Comment 8•12 years ago
|
||
Actually, second thoughts - we've had a ton of trouble with button state. shared/test/browser_toolbar_basic.js does some button state testing. Perhaps we should add something in there to check on this?
Assignee | ||
Updated•12 years ago
|
Blocks: devtools.jsm
Comment 9•12 years ago
|
||
Comment on attachment 633886 [details] [diff] [review] remove responsive mode - v1 r+, works great but I'm not browser/ peer and those two patches only touch browser/.
Attachment #633886 -
Flags: review?(cedricv) → review+
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•12 years ago
|
||
Sorry, I messed up with the review flag. I wanted Cedric to look at the style editor patch and Joe to look at the Responsive Mode patch. I will make sure the command is ok, add some test as suggested by Joe, then try tests. I will then re-ask for reviews.
Assignee | ||
Comment 12•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #633885 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #633886 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 years ago
|
||
Comment on attachment 635312 [details] [diff] [review] v2 So I have updated the tests and fixed a missing "return" in the initial patch. The gcli command will still work. Cedric already looked at the Style Editor code.
Attachment #635312 -
Flags: review?(jwalker)
Assignee | ||
Comment 14•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=f38673a3531b
Comment 15•12 years ago
|
||
Comment on attachment 635312 [details] [diff] [review] v2 Review of attachment 635312 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/styleeditor/styleeditor.xul @@ +128,5 @@ > let chrome = new StyleEditorChrome(chromeRoot, contentWindow); > chrome.selectStyleSheet(args.selectedStyleSheet, args.line, args.col); > window.styleEditorChrome = chrome; > +]]> > +</xul:script> Did you mean to change this? It's the only change in this file. I'm not sure it's actually a problem or anything.
Attachment #635312 -
Flags: review?(jwalker) → review+
Assignee | ||
Comment 16•12 years ago
|
||
thanks Joe. fixed
Assignee | ||
Updated•12 years ago
|
Attachment #635312 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Whiteboard: [land-in-fx-team]
Assignee | ||
Updated•12 years ago
|
Whiteboard: [land-in-fx-team]
Assignee | ||
Comment 17•12 years ago
|
||
Comment on attachment 635759 [details] [diff] [review] v2.1 I need a browser review for this.
Attachment #635759 -
Flags: review?(dao)
Comment 18•12 years ago
|
||
Comment on attachment 635759 [details] [diff] [review] v2.1 >+ toggle: function SE_toggle() >+ { >+ let contentWindow = gBrowser.selectedBrowser.contentWindow; >+ this.StyleEditorManager.toggleEditor(contentWindow); this.StyleEditorManager.toggleEditor(gBrowser.contentWindow); >+ refreshCommand: function SEM_refreshCommand() { >+ let contentWindow = this.chromeWindow.gBrowser.selectedBrowser.contentWindow; let contentWindow = this.chromeWindow.gBrowser.contentWindow; >+ listenToTabs: function SEM_listenToTabs() { >+ let win = this.chromeWindow; >+ let tabs = win.gBrowser.tabContainer; >+ >+ let bound_refreshCommand = this.refreshCommand.bind(this); >+ tabs.addEventListener("TabSelect", bound_refreshCommand, true); >+ >+ win.addEventListener("DOMWindowClose", function onClose(aEvent) { >+ tabs.removeEventListener("TabSelect", bound_refreshCommand, true); >+ win.removeEventListener("DOMWindowClose", onClose, false); >+ }, false); >+ }, s/DOMWindowClose/unload/
Attachment #635759 -
Flags: review?(dao) → review+
Assignee | ||
Comment 19•12 years ago
|
||
Addressed Dao's comments
Assignee | ||
Updated•12 years ago
|
Attachment #635759 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Whiteboard: [land-in-fx-team]
Assignee | ||
Updated•12 years ago
|
Whiteboard: [land-in-fx-team]
Assignee | ||
Comment 20•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e51851dba7a2
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 21•12 years ago
|
||
Backed out: https://hg.mozilla.org/integration/fx-team/rev/cf6d687f0784 Leaks: https://tbpl.mozilla.org/php/getParsedLog.php?id=12965228&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=12965059&tree=Fx-Team ...
Whiteboard: [fixed-in-fx-team]
Comment 22•12 years ago
|
||
The two changesets are in m-c, too: https://hg.mozilla.org/mozilla-central/rev/e51851dba7a2 https://hg.mozilla.org/mozilla-central/rev/cf6d687f0784
Assignee | ||
Comment 23•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #636257 -
Attachment is obsolete: true
Assignee | ||
Comment 24•12 years ago
|
||
The Map was holding a refence to the editors and the contentWindows. Fixed in attachment 636382 [details] [diff] [review] And it's green: https://tbpl.mozilla.org/?tree=Try&rev=8d0a5ef977ac
Assignee | ||
Comment 25•12 years ago
|
||
(I just use an weakmap instead. I love weakmaps)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [land-in-fx-team]
Comment 26•12 years ago
|
||
Comment on attachment 636382 [details] [diff] [review] [in-fx-team] v2.3 https://hg.mozilla.org/integration/fx-team/rev/d9e78d7def12
Attachment #636382 -
Attachment description: v2.3 → [in-fx-team] v2.3
Updated•12 years ago
|
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment 27•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d9e78d7def12
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•