Closed Bug 765564 Opened 12 years ago Closed 12 years ago

[devtb] Add a DevTools menu to the developer toolbar

Categories

(DevTools :: General, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 17

People

(Reporter: paul, Assigned: paul)

References

Details

Attachments

(2 files, 8 obsolete files)

We need a dropdown menu with the tools that are not present in the toolbar.
Blocks: 745773
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.
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.
(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!
Attached patch v1 (obsolete) — Splinter Review
Depends on: 764555
Attached patch v1.1 (obsolete) — Splinter Review
Attachment #636750 - Attachment is obsolete: true
Attachment #636767 - Flags: review?(dcamp)
Attached patch v1.2 (obsolete) — Splinter Review
better position for the popup
Attachment #636767 - Attachment is obsolete: true
Attachment #636767 - Flags: review?(dcamp)
No longer blocks: 745773
Attached patch v1.3 (obsolete) — Splinter Review
Attachment #636814 - Attachment is obsolete: true
Attached patch v1.5 (obsolete) — Splinter Review
Attachment #640219 - Attachment is obsolete: true
Attachment #640268 - Flags: review?(dao)
Blocks: 759102
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.
Attachment #640268 - Flags: review?(dao) → review-
Would it make more sense to just reparent the menu on popup, and then put it back after it closes?
(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.
Or we could just put the wanted menu items directly in the markup.
(In reply to Dão Gottwald [:dao] from comment #12)
> Or we could just put the wanted menu items directly in the markup.

ok
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.
s/should code/should share code/
Attached patch v2 (obsolete) — Splinter Review
Attachment #640268 - Attachment is obsolete: true
Comment on attachment 641489 [details] [diff] [review]
v2

Forgot to ask for the review… dumb me.
Attachment #641489 - Flags: review?(dao)
Attached patch v2.1 (obsolete) — Splinter Review
forgot to remove some debug stuff
Attachment #641489 - Attachment is obsolete: true
Attachment #641489 - Flags: review?(dao)
Attachment #641780 - Flags: review?(dao)
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.
review ping
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
Attachment #641780 - Flags: review?(dao) → review+
Attached patch v2.2 (obsolete) — Splinter Review
Attachment #641780 - Attachment is obsolete: true
Whiteboard: [land-in-fx-team]
Blocks: 754481
https://hg.mozilla.org/integration/fx-team/rev/486fd8f6d29d
Assignee: nobody → paul
Status: NEW → ASSIGNED
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
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
Whiteboard: [fixed-in-fx-team]
Attached patch v2.3Splinter Review
removed the accesskey for the error console and page source
Attachment #642629 - Attachment is obsolete: true
https://tbpl.mozilla.org/?tree=Fx-Team&rev=b658834ae434
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b658834ae434
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 17
Depends on: 776254
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.
(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?
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: