142961 - Inconsistency between Edit and Tools menu

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P3)

Description

[Henrik Gemal]

When the "Fill in Form" menuitem is disabled in the Edit menu the same item in
Tools -> Form Manager is not disabled.

20020507

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This should be a piece of cake.

Reassigning to new module owner.

Comment 2

[Andreas Kunz]

Attachment: patch - make IDs unique

This bug is because those two menu entries in walletNavigatorOverlay.xul have
the same ID ("edit_prefill") and changing the state happens after fetching the
element(s) by ID, so only one of them is changed.
This patch makes the IDs unique, which fixes this bug.
The same problem is with ID "edit_capture", this is also fixed here.

Comment 3

[Andreas Kunz]

Comment on attachment 139869 [details] [diff] [review]
patch - make IDs unique

Requesting r= from Neil.

An alternative approach might be to change setDisabledAttr to not take any
parameters and let it automatically set prefill and capture states - it is only
called from those lines in the patch, 2/4 times consecutively. (plus one time
from commented out code)

Comment 4

[[e-mail]]

Comment on attachment 139869 [details] [diff] [review]
patch - make IDs unique

Did you not wonder about the name initEditItems?

Comment 5

[Andreas Kunz]

Attachment: patch using a commandset

This might be cleaner.

Comment 6

[Andreas Kunz]

Comment on attachment 140105 [details] [diff] [review]
patch using a commandset

Requesting r= from Neil.

There's a lot of commented-out code that was the Forms Toolbar in some distant
past (already inactive when that code was checked in at Feb 20 2001).

I did not touch it, which is solution ) out of:
a) remove it
b) ignore it
c) modify it to "work" with my changes

Comment 7

[[e-mail]]

Comment on attachment 140105 [details] [diff] [review]
patch using a commandset

>+      <command id="prefillCmd" oncommand="formPrefill();"/>
>+      <command id="captureCmd" oncommand="formCapture();"/>
Nits:

If you search LXR you'll find that most commands are named cmd_xxx
You can also put the label and the access key on the command.

Comment 8

[Andreas Kunz]

Attachment: patch, nits addressed plus accesskey changed

Using "cmd_walletPrefill" because cmd_PrefillForm is already used and I want to
reduce possible confusion.
However, I discovered that the accesskey ("F") is also used by the "Find"
menuitem in the "Edit" menu. So there walletPrefill cannot be called.
I changed the accesskey to "i" which is not used - in both places. However, if
this is not desired, I could omit that change.

Comment 9

[Andreas Kunz]

Comment on attachment 140176 [details] [diff] [review]
patch, nits addressed plus accesskey changed

Requesting r= from Neil.

Comment 10

[[e-mail]]

Comment on attachment 140176 [details] [diff] [review]
patch, nits addressed plus accesskey changed

Good catch on the "F" access key but "i" isn't much better because not only is
it narrow but it is a vowel. r=me if you use "m" instead.

Comment 11

[Andreas Kunz]

Attachment: the same patch, accesskey is now "n"

Neil, I can't use "m" because "Manage sites" has that one already (in the Tools
submenu). The only choice left seems to be "n" which does not look so bad to me
after all. This patch does that. Ok?

Comment 12

[Andreas Kunz]

Comment on attachment 140281 [details] [diff] [review]
the same patch, accesskey is now "n"

Got Neil's r= for "n" on IRC, now requesting sr= from alecf.

Comment 13

[[e-mail]]

Comment on attachment 140281 [details] [diff] [review]
the same patch, accesskey is now "n"

After all that I forgot to double check the Tools menu :-[

Comment 14

[Alec Flett]

Comment on attachment 140281 [details] [diff] [review]
the same patch, accesskey is now "n"

sr=alecf

Comment 15

[[e-mail]]

Fix checked in.