Closed
Bug 917072
Opened 11 years ago
Closed 11 years ago
move black box eyeball into sources toolbar
Categories
(DevTools :: Debugger, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 27
People
(Reporter: fitzgen, Assigned: fitzgen)
References
(Depends on 1 open bug)
Details
Attachments
(4 files, 3 obsolete files)
60.10 KB,
patch
|
vporof
:
review+
|
Details | Diff | Splinter Review |
184.35 KB,
image/png
|
Details | |
177.21 KB,
image/png
|
Details | |
1.81 KB,
patch
|
past
:
review+
|
Details | Diff | Splinter Review |
The black box eyeball shouldn't be next to every single source anymore, it should be a toggle-able button in the sources toolbar.
Comment 1•11 years ago
|
||
How would you target individual sources? (don't say via the commandline).
Assignee | ||
Comment 2•11 years ago
|
||
The selected source would be the one black boxed (or un-black boxed).
Assignee | ||
Updated•11 years ago
|
Depends on: dbg-blackbox
Comment 3•11 years ago
|
||
So I need to first wait for jquery to load and be displayed, to be able to blackbox it? A bit sad. But I guess this would be fine after having a blackbox source pragma, or blackboxing minified sources by default.
Assignee | ||
Comment 4•11 years ago
|
||
Well, we can't currently black box a source until it is loaded in the sources list, this doesn't change that. You don't have to wait for the whole source text to be loaded. Unfortunately, it does add one extra click: select source then click eyeball, instead of hover over source and click eyeball. I don't see a way we can keep adding features that require UI without either making the UI really crowded / maximalistic, or by hiding things until you are in their context (which often requires one extra click, like it does here).
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → nfitzgerald
Priority: -- → P2
Assignee | ||
Comment 6•11 years ago
|
||
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Comment 8•11 years ago
|
||
Haven't updated tests to work with the new button, or the non OSX CSS. The black box eyeball is a little big for the button, and there is like 1 pixel on each side being cut off. I don't think there is something like background-size for list-style-image, so I will probably have to find the smaller eyeball that shorlander sent me.
Assignee | ||
Comment 9•11 years ago
|
||
Would really like to land this before uplift! :) Deleted the old browser_dbg_blackboxing-06.js because it no longer makes sense. Moved 07 to 06. Using smaller eyeyball icon image (although it is displayed as the same size it was before, because we were scaling it down with CSS). Can't continue to scale in CSS because toolbarbuttons use list-style-image for their icons, not background-image. Simplified a lot of test stuff by moving shared code into head.js.
Attachment #819269 -
Attachment is obsolete: true
Attachment #819270 -
Attachment is obsolete: true
Attachment #819271 -
Attachment is obsolete: true
Attachment #820791 -
Flags: review?(vporof)
Assignee | ||
Comment 10•11 years ago
|
||
Assignee | ||
Comment 11•11 years ago
|
||
Comment 12•11 years ago
|
||
Comment on attachment 820791 [details] [diff] [review] bug-917072-bb-sources-toolbar.patch Review of attachment 820791 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/debugger/debugger-panes.js @@ +709,2 @@ > */ > + updateButtonState: function() { Rename this to updateToolbarButtonsState ('cuz they're more than 1). @@ +734,5 @@ > DebuggerView.Filtering.target = this; > }, > > /** > * The check listener for the sources container. This comment is now incorrect. ::: browser/devtools/debugger/debugger.xul @@ +321,5 @@ > <hbox id="debugger-widgets" flex="1"> > <vbox id="sources-pane"> > <vbox id="sources" flex="1"/> > <toolbar id="sources-toolbar" class="devtools-toolbar"> > + <hbox id="sources-controls"> Nit: the extra hbox is premature at this point, but I wouldn't necessarily remove it. ::: browser/devtools/debugger/test/browser_dbg_blackboxing-01.js @@ +26,5 @@ > }); > } > > function testBlackBoxSource() { > + const bbButton = getBlackBoxButton(gPanel); bee bee buttun ::: browser/devtools/debugger/test/head.js @@ +537,5 @@ > + > + if (aSource) { > + aPanel.panelWin.DebuggerView.Sources.selectedValue = aSource; > + ensureSourceIs(aPanel, aSource, true) > + .then(clickBlackBoxButton); Nit: .then would look better on the same line IMO. ::: browser/locales/en-US/chrome/browser/devtools/debugger.dtd @@ +33,5 @@ > - the button that opens up an options context menu for the debugger UI. --> > <!ENTITY debuggerUI.optsButton.tooltip "Debugger Options"> > > +<!-- LOCALIZATION NOTE (debuggerUI.sources.blackBoxTooltip): This is the tooltip for the > +button that black boxes the selected source. --> Uber nit: localization comment doesn't follow the other comments' pattern. (Move the 'the' on the next line and fix indentation). ::: browser/themes/linux/devtools/debugger.css @@ -50,5 @@ > - background-size: 32px 16px; > - background-position: -16px 0; > - width: 16px; > - height: 16px; > - border: 0; WOOHOOOO!! @@ +32,3 @@ > } > > ++#sources .side-menu-widget-item-contents.black-boxed { There's a + at the beginning of this line. U rebase by hand bro? @@ +35,4 @@ > color: #888; > } > > +#sources .side-menu-widget-item-contents.black-boxed > .dbg-breakpoint { Is .side-menu-widget-item-contents still necessary? (Ditto on the above selector). ::: browser/themes/osx/devtools/debugger.css @@ +37,4 @@ > color: #888; > } > > +#sources .side-menu-widget-item-contents.black-boxed > .dbg-breakpoint { Ditto. ::: browser/themes/windows/devtools/debugger.css @@ +35,4 @@ > color: #888; > } > > +#sources .side-menu-widget-item-contents.black-boxed > .dbg-breakpoint { Ditto.
Attachment #820791 -
Flags: review?(vporof) → review+
Assignee | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/2ed8270e713f
Whiteboard: [fixed-in-fx-team]
Comment 14•11 years ago
|
||
Attachment #820887 -
Flags: review?(past)
Updated•11 years ago
|
Attachment #820887 -
Flags: review?(past) → review+
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2ed8270e713f https://hg.mozilla.org/mozilla-central/rev/01672d68f16a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•