[cust] Creating a new toolbar with same name as standard toolbar should give an error

VERIFIED FIXED in Firefox1.0beta

Status

()

Firefox
Toolbars and Customization
P4
minor
VERIFIED FIXED
15 years ago
11 years ago

People

(Reporter: David McCormack, Assigned: Blake Ross)

Tracking

({fixed-aviary1.0})

unspecified
Firefox1.0beta
x86
Windows XP
fixed-aviary1.0
Points:
---
Bug Flags:
blocking0.9 -
blocking-aviary1.0 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20020927 Phoenix/0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20020927 Phoenix/0.2

Create a new toolbar called 'Bookmark Toolbar'. No error is reported that a
toolbar with that name already exists and a new toolbar and menu item are created.


Reproducible: Always

Steps to Reproduce:


Actual Results:  
Phoenix creates the toolbar and you end up with duplicate menu items on the
Toolbars menu.

Expected Results:  
If you try and create a second user generated toolbar with the same name as a
previous user generated toolbar an error is reported and you are prompted to
change the name. This should also happen with the 'Status Bar', 'Navigation
Toolbar' and 'Bookmarks Toolbar' names.


The new toolbar facilities are excellent, it means an end to having alter
browser.xul to get an 'Address toolbar'.

Comment 1

15 years ago
confirmed. not sure what we want to do about this. over to hewitt to decide. 
Assignee: hyatt → hewitt
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Creating a new toolbar with same name as standard toolbar should give an error → [cust] Creating a new toolbar with same name as standard toolbar should give an error

Updated

15 years ago
Severity: normal → minor

Comment 2

15 years ago
Also, the default 2 toolbar names can not be changed. Try this:

1. Customize toolbar
2. Move all the buttons/address bar/search bar onto the Menu Bar, next to "Help"
3. So now the "Navigation Bar" is completely empty.

ACTUAL results:
The empty "Navigation Bar" is still called "Navigation Bar". This is confusing
if you want to check them on/off in View->ToolBars. Same goes for "Bookmark Bar".

Solutions:

* Allow renaming of default toolbars.

* "Grips" in toolbars (Bug #172818) so you can move the whole Navigation Bar and
won't have this issue.

* Empty rows should not be allowed, if the 2nd row is empty, the 3rd row should
move up automatically.
Created attachment 128083 [details] [diff] [review]
Now also blocks Bookmark/Navigation toolbar.

This pacth also blocks new toolbar by their display name, taken from the list
used for the View -> Toolbars menu.

It also fixes stripping the spaces from the id's generated. It now strips all
spaces insteda of just the first one.

I suggest checking in this patch (if it passes review) and leave the other
issues to more relevant bugs.
(Assignee)

Comment 4

15 years ago
Hey Henrik, thanks for that patch.  Question: why do we still need the whole
nameToId check?  Custom toolbars have a toolbarname also.  It seems like the new
check you've added will be sufficient.
Target Milestone: --- → Firebird0.7
HI blake

In though about removeing it since it seems redundant, but on second thought I
think we do need to 

The reason is that two toolbars with different display names may generate the
same id.
All these toolbar names (without the quotes) make for the same id
"My Toolbar"
"My    Toolbar"
"   My Toolbar"
"My Toolbar "

I see two options:

1: Leave the patch as is with the double check.
2: Change the nameToId conversion to use underscorer for spaces, instead of
stripping them.

I you like #2 best, I thinkI can quickly change the patch.



Comment 6

15 years ago
Taking QA Contact
QA Contact: asa → bugzilla
Created attachment 129082 [details] [diff] [review]
Get rid og the double cheking and alter toolbar.xml to generate uniqe ids.

This patch is an update, it has two changes:
- the extra dupe check has been removed so we now only look at the display
names.
- toolkit/content/wedget/toolbar.xml has been altered to generate uniqe ids for
custom toolbars, needed for the first change to work
Attachment #128083 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Target Milestone: Firebird0.7 → Firebird0.8

Comment 8

14 years ago
Does this patch also prevent the creation of toolbars with no name? If you guys
agree that you shouldn't be able to do that, a fix at the same time would be
handy, per bug 214707. Thanks :)
I'm giving this one to blake, as he's more likely to review and check this in
than hewitt is. 
Assignee: hewitt → blake
-> 0.9
Target Milestone: Firebird0.8 → Firebird0.9

Updated

14 years ago
Flags: blocking0.9?
not a blocker for 0.9, maybe 1.0
Flags: blocking0.9? → blocking0.9-

Updated

14 years ago
Flags: blocking1.0?
+ing since this has a patch. 
Flags: blocking1.0? → blocking1.0+
Priority: -- → P4
Target Milestone: Firefox0.9 → Firefox1.0beta
(Assignee)

Comment 13

14 years ago
Fixed.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 14

14 years ago
Note that my patch to bug 180164 will give the 'menu bar' a displayable name, so
the type check in the following line will have to go.

+            if (toolbarName==name.value && type != "menubar") {

Thanks,
David

Comment 15

14 years ago
On what build(s) is this fixed? I can still create a second "Bookmarks Toolbar"
on the 20040715 nightly build on the Mac, so the bug still seems to exist. I
can't check Windows nightlies, since they're still down.

Comment 16

14 years ago
I apologize. It seems that the original intent of this report was to make it so
that two toolbars with the same name can't exist, but it's been changed to allow
duplicate names, but assign each a unique id for the code to handle. So I
presume it is fixed afterall, even though I can create a second "Bookmarks
Toolbar" in the latest Win XP nightly.

Updated

13 years ago
Keywords: fixed-aviary1.0
Status: RESOLVED → VERIFIED

Updated

11 years ago
QA Contact: bugzilla → toolbars
You need to log in before you can comment on or make changes to this bug.