64355 - form manager toolbar should be disabled by default

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[Christopher Blizzard (:blizzard)]

I think that the form manager toolbar should be disabled by default.  It's not
used by name people and the fact that it vanishes and re-appears is awful UI.
Can we make it off by default?  I'll attach a patch in a moment.

Comment 1

[Christopher Blizzard (:blizzard)]

Attachment: disable toolbar

Comment 2

[Alec Flett]

that's all we need? cool. sr=alecf

Comment 3

[Stephen P. Morse]

Hold off please.  I'm currently looking at this and it's not that simple.

Comment 4

[Blake Ross]

Why are |checked| and |type| attributes of the broadcaster instead of the 
menuitem in this case?  That doesn't seem right.

Comment 5

[Stephen P. Morse]

Attachment: Patch to have form toolbar off by default

Comment 6

[Blake Ross]

onload="alert('hi');"?

There are many attributes on the <broadcaster/> that belong on the <menuitem/>, 
including class, type and checked.

Comment 7

[Stephen P. Morse]

Oop, debugging code that I forgot to remove.

Comment 8

[Stephen P. Morse]

Attachment: same patch but with debugging code removed

Comment 9

[Stephen P. Morse]

In a private message to me, blizzard wrote:

   Looks reasonable to me.  I didn't know that it required that much. 
   sr=blizzard

So all we need now is a normal review.

Comment 10

[Blake Ross]

I would review if those misplaced attributes were moved to the menuitem...

Comment 11

[Stephen P. Morse]

Those "misplaced" attributes appear on all the other broadcasters that already 
exist in the view menu.  So if you object to that, you should open a new bug to 
have them removed.  That is not the topic of this bug report.

Comment 12

[Eric Vaughan]

r=evaughan

Comment 13

[Stephen P. Morse]

Fix checked in.

Comment 14

[Stephen P. Morse]

There were three postings to bug 23095 that really belong here instead.  They 
are as follows:

------- Additional Comments From Mark Olson 2001-01-05 00:15 -------

This checkin also causes several new javascript strict errors in navigator.js.
Also, there is some suspicious code in setFormToolbar such as:

  var formToolbar = document.getElementById("FormToolbar");
  if (!formToolbar) {
    formToolbar.setAttribute("hidden", "true");
    return;
  }

where formToolbar is referenced after we know it is invalid, and line 1671 where
formToolbar is referenced before being declared.



------- Additional Comments From [e-mail] 2001-01-05 01:45 -------

I guess I was a bit overzelous here with my hiding code.  Attaching a patch to 
fix this.

Treating this as a bustage (introduced javascript errors) and checking it in the 
fix immediately.



------- Additional Comments From [e-mail] 2001-01-05 01:45 -------

Created an attachment (id=21826)
patch to fix bustage

Comment 15

[Stephen P. Morse]

Of course you'll have to go to bug 23095 to see the patch just mentioned (21826) 
because the above is copied text and not a real link.

Comment 16

[Terri Preston]

Verified Windows 2001020804
Verified Linux 2001020808
Verified Mac 2001020804