Last Comment Bug 765564 - [devtb] Add a DevTools menu to the developer toolbar
: [devtb] Add a DevTools menu to the developer toolbar
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 17
Assigned To: Paul Rouget [:paul]
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on: 764555 776254
Blocks: 754481 759102
  Show dependency treegraph
 
Reported: 2012-06-17 04:28 PDT by Paul Rouget [:paul]
Modified: 2012-08-30 09:58 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (6.25 KB, patch)
2012-06-26 09:43 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v1.1 (8.02 KB, patch)
2012-06-26 10:18 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v1.2 (8.06 KB, patch)
2012-06-26 11:54 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v1.3 (8.19 KB, patch)
2012-07-09 08:00 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v1.5 (8.06 KB, patch)
2012-07-09 10:20 PDT, Paul Rouget [:paul]
dao+bmo: review-
Details | Diff | Splinter Review
v2 (33.49 KB, patch)
2012-07-12 08:31 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v2.1 (33.45 KB, patch)
2012-07-13 02:32 PDT, Paul Rouget [:paul]
dao+bmo: review+
Details | Diff | Splinter Review
v2.2 (33.83 KB, patch)
2012-07-16 10:16 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v2.3 (33.65 KB, patch)
2012-07-19 17:23 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
More Tools: checkmarks are not treated equally (7.16 KB, image/png)
2012-07-22 03:34 PDT, Alfred Kayser
no flags Details

Description Paul Rouget [:paul] 2012-06-17 04:28:27 PDT
We need a dropdown menu with the tools that are not present in the toolbar.
Comment 1 Kevin Dangoor 2012-06-18 05:59:53 PDT
Early mockups of the toolbar had a star (or some icon like that), which I believe was also the idea for Firefox's own "20%" features (limi's term for the stuff used 20% of the time/or by 20% of the users).

I'm curious what presentation you have in mind now. I'm thinking it's probably not literally a "DevTools" menu as listed in the summary of this bug.
Comment 2 Paul Rouget [:paul] 2012-06-18 06:08:31 PDT
I haven't done anything yet for this bug, and I need to think about the UI.

I was thinking about a menu like the Bookmark menu.

My plan is to use the same button that we use to list the scripts in the debugger, but instead of text, I'll use a "tool" icon.

This is the simplest way to get all the tools in the devtb at the moment. But I am open to any suggestion.
Comment 3 Kevin Dangoor 2012-06-18 07:15:13 PDT
(In reply to Paul Rouget [:paul] from comment #2)
> My plan is to use the same button that we use to list the scripts in the
> debugger, but instead of text, I'll use a "tool" icon.

Sounds good. Simple to start works for me!
Comment 4 Paul Rouget [:paul] 2012-06-26 09:43:49 PDT
Created attachment 636750 [details] [diff] [review]
v1
Comment 5 Paul Rouget [:paul] 2012-06-26 10:18:37 PDT
Created attachment 636767 [details] [diff] [review]
v1.1
Comment 6 Paul Rouget [:paul] 2012-06-26 11:54:18 PDT
Created attachment 636814 [details] [diff] [review]
v1.2

better position for the popup
Comment 7 Paul Rouget [:paul] 2012-07-09 08:00:23 PDT
Created attachment 640219 [details] [diff] [review]
v1.3
Comment 8 Paul Rouget [:paul] 2012-07-09 10:20:43 PDT
Created attachment 640268 [details] [diff] [review]
v1.5
Comment 9 Dão Gottwald [:dao] 2012-07-10 15:08:14 PDT
Comment on attachment 640268 [details] [diff] [review]
v1.5

>+          <toolbarbutton id="developer-toolbar-menu"

I'd prefer a more meaningful id such as "developer-toolbar-other-tools".

>+                         label="&devToolbarMenuButton.label;">

ditto

>+  let reference = this._doc.getElementById("menuWebDeveloperPopup");
>+  let popup = this._doc.getElementById("developer-toolbar-menu").firstChild;
>+  for (let item of reference.childNodes) {
>+    let command = item.getAttribute("command");
>+
>+    if (command) {
>+      let selector = "#developer-toolbar > toolbarbutton[command=\"" + command + "\"]";
>+      let button = this._doc.querySelector(selector);
>+      let isInToolbar = button && !button.hidden;
>+      if (isInToolbar) {
>+        continue;
>+      }
>+    }
>+
>+    popup.appendChild(item.cloneNode());
>+  }

This clones menuWebDeveloperPopup's children including their id, so we end up with duplicate ids in the document.
Comment 10 Dave Camp (:dcamp) 2012-07-10 15:10:13 PDT
Would it make more sense to just reparent the menu on popup, and then put it back after it closes?
Comment 11 Paul Rouget [:paul] 2012-07-10 15:44:59 PDT
(In reply to Dave Camp (:dcamp) from comment #10)
> Would it make more sense to just reparent the menu on popup, and then put it
> back after it closes?

But then you'll see the elements that are hidden in the toolbar.

We could just remove the id. Maybe while cloning, or better, directly from original menuitems.
Comment 12 Dão Gottwald [:dao] 2012-07-10 16:04:12 PDT
Or we could just put the wanted menu items directly in the markup.
Comment 13 Paul Rouget [:paul] 2012-07-10 16:08:44 PDT
(In reply to Dão Gottwald [:dao] from comment #12)
> Or we could just put the wanted menu items directly in the markup.

ok
Comment 14 Paul Rouget [:paul] 2012-07-11 06:30:19 PDT
We are reproducing the same code (menu popup) in 3 different places (appmenu, menubar, devtb menu) and maybe 4 (bug 754481).

We can't use the same popups (because the content is not exactly the same), so I think we should code via some broadcasters.
Comment 15 Paul Rouget [:paul] 2012-07-11 06:30:42 PDT
s/should code/should share code/
Comment 16 Paul Rouget [:paul] 2012-07-12 08:31:34 PDT
Created attachment 641489 [details] [diff] [review]
v2
Comment 17 Paul Rouget [:paul] 2012-07-13 02:28:42 PDT
Comment on attachment 641489 [details] [diff] [review]
v2

Forgot to ask for the review… dumb me.
Comment 18 Paul Rouget [:paul] 2012-07-13 02:32:53 PDT
Created attachment 641780 [details] [diff] [review]
v2.1

forgot to remove some debug stuff
Comment 19 Paul Rouget [:paul] 2012-07-13 02:47:14 PDT
Dao, if you think you can't review this patch in time for the Aurora merge, I can write a simpler patch without this XUL refactoring.
Comment 20 Paul Rouget [:paul] 2012-07-14 09:55:22 PDT
review ping
Comment 21 Dão Gottwald [:dao] 2012-07-16 07:11:28 PDT
Comment on attachment 641780 [details] [diff] [review]
v2.1

>+<!ENTITY devToolbarOtherToolsButton.label  "More tools">

"More Tools"

Please rename all broadcaster:Tools:Foo instances to devtoolsMenuBroadcaster_Foo.

r=me with that
Comment 22 Paul Rouget [:paul] 2012-07-16 10:16:01 PDT
Created attachment 642629 [details] [diff] [review]
v2.2
Comment 23 Panos Astithas [:past] 2012-07-18 07:10:09 PDT
https://hg.mozilla.org/integration/fx-team/rev/486fd8f6d29d
Comment 24 Panos Astithas [:past] 2012-07-18 09:48:44 PDT
Backed out:
https://hg.mozilla.org/integration/fx-team/rev/7b3b0c5c9933

Apparently it breaks browser_bug616836.js on Linux:
https://tbpl.mozilla.org/?tree=Fx-Team&rev=564ed4fde8ab
Comment 25 Ed Morley [:emorley] 2012-07-18 10:26:39 PDT
Also on Windows:
https://tbpl.mozilla.org/php/getParsedLog.php?id=13640840&tree=Fx-Team
Comment 26 Paul Rouget [:paul] 2012-07-19 16:30:05 PDT
https://tbpl.mozilla.org/?tree=Try&rev=2d1ee962a1e0
Comment 27 Paul Rouget [:paul] 2012-07-19 17:23:13 PDT
Created attachment 644079 [details] [diff] [review]
v2.3

removed the accesskey for the error console and page source
Comment 28 Paul Rouget [:paul] 2012-07-20 15:24:55 PDT
https://tbpl.mozilla.org/?tree=Fx-Team&rev=b658834ae434
Comment 29 Tim Taubert [:ttaubert] 2012-07-21 03:51:39 PDT
https://hg.mozilla.org/mozilla-central/rev/b658834ae434
Comment 30 Alfred Kayser 2012-07-22 03:34:54 PDT
Created attachment 644743 [details]
More Tools: checkmarks are not treated equally

The checkmarks of the More Tools menu are not treated equally.
Only Developer Toolbar, Responsive Design View and Style Editor get a checkmark, the others not. Those with checkmark can also close the corresponding window/dialog, but the others not.

It would be much nicer if all these windows can be opened and closed in the same way.
Comment 31 Paul Rouget [:paul] 2012-08-30 09:58:28 PDT
(In reply to Alfred Kayser from comment #30)
> Created attachment 644743 [details]
> More Tools: checkmarks are not treated equally
> 
> The checkmarks of the More Tools menu are not treated equally.
> Only Developer Toolbar, Responsive Design View and Style Editor get a
> checkmark, the others not. Those with checkmark can also close the
> corresponding window/dialog, but the others not.
> 
> It would be much nicer if all these windows can be opened and closed in the
> same way.

Could you file a bug for that?

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