Closed Bug 142961 Opened 22 years ago Closed 20 years ago

Inconsistency between Edit and Tools menu

Categories

(Toolkit :: Form Manager, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: durbacher)

Details

Attachments

(1 file, 3 obsolete files)

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

20020507
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
Target Milestone: mozilla1.0.1 → mozilla1.1alpha
Summary: Inconsistency in Edit an Tools menu → Inconsistency between Edit and Tools menu
Priority: -- → P3
Target Milestone: mozilla1.1alpha → mozilla1.2alpha
Target Milestone: mozilla1.2alpha → mozilla1.3alpha
This should be a piece of cake.

Reassigning to new module owner.
Assignee: morse → dveditz
Status: ASSIGNED → NEW
Target Milestone: mozilla1.3alpha → ---
Attached patch patch - make IDs unique (obsolete) — Splinter Review
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.
Assignee: dveditz+bmo → durbacher
Status: NEW → ASSIGNED
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)
Attachment #139869 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 139869 [details] [diff] [review]
patch - make IDs unique

Did you not wonder about the name initEditItems?
Attachment #139869 - Flags: review?(neil.parkwaycc.co.uk) → review-
Attached patch patch using a commandset (obsolete) — Splinter Review
This might be cleaner.
Attachment #139869 - Attachment is obsolete: true
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
Attachment #140105 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #140105 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #140105 - Attachment is obsolete: true
Comment on attachment 140176 [details] [diff] [review]
patch, nits addressed plus accesskey changed

Requesting r= from Neil.
Attachment #140176 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #140176 - Flags: review?(neil.parkwaycc.co.uk) → review+
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?
Attachment #140176 - Attachment is obsolete: true
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.
Attachment #140281 - Flags: superreview?(alecf)
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 :-[
Attachment #140281 - Flags: review+
Comment on attachment 140281 [details] [diff] [review]
the same patch, accesskey is now "n"

sr=alecf
Attachment #140281 - Flags: superreview?(alecf) → superreview+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Toolkit
QA Contact: tpreston → form.manager
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: