180164 - Make menu position customizable.

Status: RESOLVED INVALID

(Firefox :: Toolbars and Customization, enhancement)

Description

[djk]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3a) Gecko/20021112 Phoenix/0.4
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3a) Gecko/20021112 Phoenix/0.4

This is sort of building on the idea of bug 177343.

I'd like to suggest the ability to move the menu to any toolbar, with 2 conditions:
1) The menu may not be placed on the Customize Toolbar palette (it must be on a
toolbar somewhere).
2) Whichever toolbar the menu is moved to, may not be hidden by View->Toolbars

Reproducible: Always

Steps to Reproduce:

Comment 1

[David P James]

How does this bug differ in any substantive way from bug 177343? As I understand
it, this bug has the additional request of being able to move the menu element
to any toolbar and not just along the menubar. If this is the case then this bug
will have to be marked as a dupe of bug 177343... one can always add a comment
to an existing RFE provided that the comment has some bearing or relationship to
the original RFE, as would certainly be the case here; that is in fact how
Phoenix ended up having the ability to move custom elements onto the menubar in
the first place.

Comment 2

[David Tenser [:djst]]

> How does this bug differ in any substantive way from bug 177343?

Bug 177343 is a bug report about the DND indicator still being used when
dragging the menu. 

This bug is a request to make the menu movable again. That feature was removed
for a reason, so this will most probably be WONTFIX'ed as soon as a developer
reads it.

Comment 3

[djk]

First, the reason I didn't just add a comment to bug 177343 is that this is a
new feture request.  Bug 177343 is tweaking/correcting the behavior of existing
functionality.


What was the reason for making the menu immobile?

I feel that the conditions that I listed above remove the major problem of menu
movement: (accidently) hiding the menu, with no easy/obvious way to get it back.

But I guess it's up to the developers. :-}

Comment 4

[David P James]

I did not know that this feature had been removed (or why, needless to say).
Given that, I'm going to mark it as WONTFIX [a little reluctantly].

Comment 5

[djk]

Mr. James,

I believe that the movable menu was an unintended side-effect of making the menu
toolbar a target for customization.  I'm not too sure why this feature was
removed.  That's why I opened this bug; to ask for a proper implementation of a
movable menu.

In comment 3, I listed why I think the menu was made immobile.  Perhaps I'm
wrong, and the developers had other reasons.  I'd like to know the reasons.  As
long as there is some rational reason why this bug is WONTFIX, I can live with it.

I ranted in the MozillaZine forum about WONTFIX:
http://www.mozillazine.org/forums/viewtopic.php?p=5558#5558

Comment 6

[David P James]

I based my WONTFIX on the fact that the developers had apparently done something
intentionally as indicated by David Tenser in comment #2. Unfortunately neither
he nor I actually know what that reason is. So I'll reopen it and see what comes
of it.

-->Reopening

Comment 7

[David P James]

-->Confirming as New for developer review

Comment 8

[djk]

Attachment: I don't know xul, but I can hack on it...

This implements the ability to move the menu items as a group.

You may not drop the menu onto the palette; it must be a toolbar drop target.

You may not hide the toolbar that the menu is on.  The toolbar name shows up in
view->toolbars, but it is disabled.  If you move the menu to another toolbar,
the previous one becomes hidable, and the new one becomes unhidable.

I don't know css/xul well enough to make the menu items 'collapse' when
customizing them (like the bookmarks currently do).  I leave it up to someone
else to implement that (or if I get mucho more time to mess around with it).

Comment 9

[djk]

BTW, cdn's Compact Menu (http://cdn.mozdev.org/compact) works with this patch, too.

Nice.

Comment 10

[djk]

Attachment: don't explicitly hide menubar in fullscreen toggle

Turns out that the menu bar would disappear when going to fullscreen mode, even
if it was located on the nav-toolbar.

Took out the line that explicitly called the hide function for the menubar, as
the menubar is implicitly hidden when its toolbar is hidden.

Comment 11

[:deanis74]

I haven't looked at the patch at all, but I have one general comment.  Please
don't put your initials in your comments, just put the comment.  We have cvs
blame to determine who did what.

Comment 12

[Simon Paquet [:sipaq]]

Taking QA Contact

Comment 13

[djk]

Dean,

I'm sorry about the initials; I use them when developing patches so that I can
find my changes quickly when editing files.  Maybe I should find a better method.

I don't really have the time right now to update the patches, but if I find some
time, I'll do so.  Otherwise someone else will have to do the cleanup before
checkin.

At any rate, I'll refrain from putting my initials in future patches.

> Took out the line that explicitly called the hide function for the menubar, as
> the menubar is implicitly hidden when its toolbar is hidden.

FWIW, I seem to remember that this one particular problem was fixed in the meantime.

David

Comment 14

[djk]

Attachment: updated patch

This is the patch, updated to a 2004-03-03 build of Fx.

There are some issues:
1) The Bookmarks Toolbar Items disappear when you "restore default set" while
customizing.  This sounds similar to a recent (fixed) bug where the menu
disappeared after the same action.  This is not introduced by my patch; it
happens to the vanilla build as well.

2) The placement of the toolbars in the View->toolbars menu is weird.  I
suspect something happened to insertBefore and XUL elements.  No idea what's
going on here.

Aside from those things, the patch seems to work. :-)

David

Comment 15

[Brendan Eich [:brendan]]

You should be asking Ben for review here.

/be

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

I'll get around to reviewing this after feature hell. -> 1.0b

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 119182 [details] [diff] [review]
don't explicitly hide menubar in fullscreen toggle

obsolete patch

Comment 18

[Ben Goodger (use ben at mozilla dot org for email)]

Nominate for 1.0 if you can create a patch that has no issues. ;-) 

Comment 19

[djk]

Attachment: updated patch for 20040519 Fx

Here's a new patch updated to current trunk.

It has no issues (as far as I tested) beyond those of the current trunk. :-}

David

Comment 20

[Benjamin Smedberg]

This has l10n impact and we don't have QA to test new features for 1.0.

Comment 21

[Thomas]

Is this bug still alive, or should I post a new one, as we're more than one year
after the latest comment, and I'm still not able to move the menu?
The bug also have target milestone 1.0 witch is too late.

Comment 22

[djk]

(In reply to comment #21)
> Is this bug still alive, or should I post a new one, as we're more than one
> year
> after the latest comment, and I'm still not able to move the menu?
> The bug also have target milestone 1.0 witch is too late.

I haven't hacked on firefox in a while.  But as I recall, this patch ran rather nicely.  I can only imagine that the code has shifted quite a bit since I last applied the patch to my local build; it probably doesn't work anymore.

If a driver would give feedback about the possibility of this being accepted, I _may_ be able to scrape up enough time to update the patch.

David

Comment 23

[:Gavin Sharp [email: [e-mail]]]

There is no chance this can land for Firefox 2, but a very good chance that it could make Firefox 3 if you prepare the patch.

Comment 24

[Mike Connor [:mconnor]]

Useful enough, would take patch for sure if something were ready/stable.

Comment 25

[Please Ignore This Troll (Account Disabled)]

the bug was forgotten?

Comment 26

[djk]

It was, understandably, pushed aside as 1.0 was nearing completion.  It was ignored by everyone during the 2.0 cycle.  When 3.0 came around with the chance to include the patch, I had no more time to work on it.

So, yes, this bug was forgotten, and remembered with poor timing.

David

Comment 27

[Josh Triplett]

With 4.0 now released, this seems like the ideal time to revive this bug.  This issue applies to the menu bar and the new menu button.

Comment 28

[Notlost]

I can't work on the patch, but I'll 'confirm' that this issue still exists...

Comment 29

[:Gavin Sharp [email: [e-mail]]]

No longer relevant given the removal of the menu by default in australis.

Comment 30

[Josh Triplett]

> No longer relevant given the removal of the menu by default in australis.

Does the new menu button support movement between toolbars?  If so, I'd call this bug FIXED.  If not, this bug still applies to the new menu button.