Closed Bug 71743 Opened 24 years ago Closed 24 years ago

Advanced Edit dialog's "Name" and "Value" textfields should be editable menulists with prefilled choices

Categories

(Core :: DOM: Editor, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: cmanske, Assigned: cmanske)

References

Details

(Whiteboard: [dialog][advanced edit]fixed, reviewed, a=asa)

Attachments

(28 files)

(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
In each panel of Advanced Edit dialog, the "Name" and "Value" textfields should contain dropdown lists of appropriate attributes, CSS attributes, JS events, etc. This would make the dialog *much* more useable. It is also the best way we have in the short term to make Composer support accessibility guidelines, since HTML attributes such as "language" and "longdesc' would be easily available. These widgets must be "editable" menulists, so users can also type names and values not in the prefilled lists.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Depends on: 62722
Blocks: 71973
Blocks: 73167
*** Bug 47259 has been marked as a duplicate of this bug. ***
Blocks: 77427
Blocks: 71027
Adding status to track Advanced Edit dialog bugs that will be fixed with new Advanced Edit work for this bug.
Whiteboard: advanced edit
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Oops! More time than I thought. I do intend to finish this in about a week. Moving back to 0.9.1
Target Milestone: mozilla0.9.2 → mozilla0.9.1
moving to 0.9.2
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Could we even get rid of the "label" property now? Either way, sr=hewitt
Good question. I'd feel safer keeping it at this pointing, even though it is rather redundant.
sr=hewitt
Attached patch Patch for editor changes (deleted) — Splinter Review
Keywords: correctness
Priority: -- → P2
Whiteboard: advanced edit → [dialog]advanced edit
* Why can I click on Attribute and Value headers? * Why does double-clicking of Attribute or Value header select the value edit field? * We shouldn't allow users to add "align" with "char" since we don't want to support that (since layout doesn't) * Unable to choose a value from the menu (it comes up empty) for already set attributes (can't change them) * For <td> bgcolor, I don't get a popup menu/list. I expect either the colorpicker or a dropdown list with the 16 colors. * I wish the drop down list icon would disappear when no list is going to appear (or disable it?) * can't add "nowrap" without giving it a value * I'd like to see more of a highlight when I select something in the attribute/ value list (so I know what I am removing if I click "remove attribute") (I'm using Classic Mac theme if that matters) more later...
Depends on: 82652
Blocks: 68800
Ok. Ready for more testing: Use latest versions of EdAEAttributes.js and the patch attached 5/29 09:29. Also apply the 5/29 patch to bug 82652. Concerning Kathy's observations: * Why can I click on Attribute and Value headers? * Why does double-clicking of Attribute or Value header select the value edit field? Just the way xul tables work, I guess. * We shouldn't allow users to add "align" with "char" since we don't want to support that (since layout doesn't) That's layout's problem. This is 'Advanced Edit' and should support every attribute in the DTD. That's the point of the dialog. * Unable to choose a value from the menu (it comes up empty) for already set attributes (can't change them) * For <td> bgcolor, I don't get a popup menu/list. I expect either the colorpicker or a dropdown list with the 16 colors. These problem should be fixed with latest patches. * I wish the drop down list icon would disappear when no list is going to appear I'll experiment with removing the "editable" attribute to see if we can do that, but I'd be surprised if we could! * can't add "nowrap" without giving it a value Should be fixed now. * I'd like to see more of a highlight when I select something in the attribute/ value list (so I know what I am removing if I click "remove attribute") (I'm using Classic Mac theme if that matters) That issue belongs to the Themes design team. I agree -- highlight is not strong enough and doesn't look different when focused or not. mo
OS: Windows NT → All
Patch on 5/29/01 18:24 fixes more of the things brade noticed: The HTML "value" input field is a <textbox> when there's 1 or less items in the value menulist. The CSS for the selection highlighting is the default for the theme; unnecessary "ae-selection" and other advanced editor specific styling was removed. The highlighting for the "required" attributes are shown in bold in Classic, and bold+italics in Modern theme (all menuitem text is bold in Modern.) To test all of this: Apply the 18:24 patch, add the EdAEAttributes.js file to editor/ui/dialog/contents, and apply the latest patch to bug 82652.
In your latest patch, I don't see any changes to a jar.mn file. Should there be?
Yes! Same change as before -- I wonder how that happened!
No longer blocks: 77427
more comments... * tabbing is a bit odd; I can't get used to it not going to the next "control" * on the <body> I can't seem to get the dropdown list for "bgcolor" value; I seem to see this problem on other tags with other attribute value lists (alignment) * we need a prompt (???) when someone sets a name/value but then doesn't click "add"? Example: choose "id", tab, type the value, choose "class", tab, type the value... * I don't understand why some menu items are italic (modern theme)
"To test" instructions: Apply latest patch to bug 82652. Apply patch from 5/31/01 21:26. Add new file from patch 05/31/01 21:38 to editor/ui/dialogs/content.
Whiteboard: [dialog]advanced edit → [dialog]advanced edit FIX IN HAND need r=, sr=
Attached patch Updated editor patch (deleted) — Splinter Review
* in td, I can add align=Char, but there isn't an option for "char" or "charoff" in the dropdown list. This seems inconsistent to me. * it's still not intuitive to me why some things (in Modern) are "bold" and others aren't * I still think it's bizarre that a double-click on the header of the list selects the contents of the "Value" editfield. It doesn't matter which header I click on (Attribute or Value). Ideally, double-click would do the same thing as click or better yet, nothing at all. We must have some control over this stuff or the double-click wouldn't be selecting our control for the value. Double- clicking in the blank area below the attribute/value * do we really want to have things like table height in the drop down list instead of encouraging users to use inline sytle? * no dropdown list of options for "dir" attribute (I'm in <table> now); expect "rtl" and "ltr" more another time...
Any changes in actual contents of the dropdown lists is trivial to make: just edit the very readable "EdAEAttributes.js". Thus we will investigate your suggestions based on the DTD. The "bold" items in the attribute list are the "required" attributes. Should we add a message under the menulist to explain that? I'll look into supressing the double-click on heading behavior.
Rather than having "bold" (or italic) items in the menu, how about we have a button to "prefill all required attributes"?
To test this: Apply latest patch to bug 82652. Apply patch from 6/5/01 12:33 in "mozilla" directory. Add new file from patch 6/5/01 12:31 to editor/ui/dialogs/content.
Here's a few comments based on the first part of the most recent patch file (sorry I can't do more; my head is too congested): * in onChangeCSSAttribute(), you should check for (!name) before trimming the value string. * would prefer "NewCSSAttribute" begin with a verb such as "AddNewCSSAttribute" * in onSelectCSSTreeItem(), "return" should be on its own line to match up with other places in file * in RemoveCSSAttribute(), "return" should be on its own line * in SelectCSSTree(), the first line is blank (intentional?) * in BuildHTMLAttributeNameList(), forceInteger and forceOneChar don't need to be initialized until we get inside the "if" * in BuildHTMLAttributeNameList(), there is this typo in a comment: fitering * in onChangeHTMLAttribute(), you should check for (!name) before trimming the value string. * in onSelectHTMLTreeItem(), "return" should be on its own line * in onInputHTMLAttributeName(), you have "var firstItem = null;" but never use it; remove line * in onInputHTMLAttributeName(), you have declared two variables named "newValue"; rename one or both * in onInputHTMLAttributeValue(), "return" should be on its own line * in RemoveHTMLAttribute(), "return" should be on its own line * in BuildJSEAttributeTable(), "if(nodeMap.length > 0)" should have a space after the "if" * in EdAdvancedEdit.xul, is "tooltiptext" localizable? Some things to test (if you haven't already): * how do we handle the use of single and double quotes in the values? (note: this may not be any more broken than before but it'd be good to know) * why do some attributes (with no values) come in with value == attribute? Other random thoughts: * I like the idea of adding a 3rd column to the tree and showing an icon whether that attribute is required, standard (in spec), or non-standard (possibly others?) * I like the idea of having a button to "add required attributes" and giving them default values (if not already present in list)
All of the syntax and format changes suggested by brade have been applied. On the issue of "required" attributes, I removed the setting of any styles; I'd prefer to not add more columns, buttons, etc for this version (let's keep it simple!). We should explore those enhancements later. Cause of the assertions are fixed, but there is still the annoying JS error: line 0: uncaught exception: [Exception... "Component returned failure code: 0x8 0004005 (NS_ERROR_FAILURE) [nsIMenuBoxObject.activeChild]" nsresult: "0x8000400 5 (NS_ERROR_FAILURE)" location: "JS frame :: <unknown filename> :: onxblcreate :: line 5" data: no] that occurs when you open the Attribute menulist and there's no selected item in that popup yet. It doesn't seem to cause any real failures, lockups, crashes, etc. Continuing to investigate. The new file (EdAEAttributes.js) attached on 6/05/01 12:33 is still valid.
Adding dependency on bug 35847, which has caused the input field in editable menulists from using the correct style, and thus cutting off half the text.
Depends on: 35847
r=brade@netscape.com and sr=kin@netscape.com on the 06/12/01 12:25 and 06/12/01 12:29 patches!! Check this in quick before the diffs change again! :-)
Whiteboard: [dialog]advanced edit FIX IN HAND need r=, sr= → [dialog]advanced edit FIX IN HAND need a=
a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)
Whiteboard: [dialog]advanced edit FIX IN HAND need a= → [dialog][advanced edit]fixed, reviewed, a=asa
checked in. Yea!
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Charley is gonna help QA by verifying this one and marking VERIFIED-FIXED. thanks Charley!
Sujay: Not a big deal, but you can verify it, can't you? It works!
Status: RESOLVED → VERIFIED
Thanks Charley...I also verified it...just wanted to have another pair of eyes to double check because this bug had lots of attachments and dependencies.. thanks.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: