Closed
Bug 71743
Opened 24 years ago
Closed 23 years ago
Advanced Edit dialog's "Name" and "Value" textfields should be editable menulists with prefilled choices
Categories
(Core :: DOM: Editor, defect, P2)
Core
DOM: Editor
Tracking
()
VERIFIED
FIXED
mozilla0.9.2
People
(Reporter: cmanske, Assigned: cmanske)
References
Details
(Whiteboard: [dialog][advanced edit]fixed, reviewed, a=asa)
Attachments
(28 files)
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.
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Assignee | ||
Comment 2•24 years ago
|
||
Adding status to track Advanced Edit dialog bugs that will be fixed with new
Advanced Edit work for this bug.
Whiteboard: advanced edit
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Assignee | ||
Comment 3•24 years ago
|
||
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
Assignee | ||
Comment 5•24 years ago
|
||
Comment 6•24 years ago
|
||
Could we even get rid of the "label" property now? Either way, sr=hewitt
Assignee | ||
Comment 7•24 years ago
|
||
Good question. I'd feel safer keeping it at this pointing, even though it is
rather redundant.
Assignee | ||
Comment 8•24 years ago
|
||
Assignee | ||
Comment 9•24 years ago
|
||
Comment 10•24 years ago
|
||
sr=hewitt
Assignee | ||
Comment 11•24 years ago
|
||
Assignee | ||
Comment 12•24 years ago
|
||
Assignee | ||
Comment 13•24 years ago
|
||
Assignee | ||
Comment 14•24 years ago
|
||
Assignee | ||
Comment 15•24 years ago
|
||
Updated•24 years ago
|
Comment 16•24 years ago
|
||
* 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...
Assignee | ||
Comment 17•23 years ago
|
||
Assignee | ||
Comment 18•23 years ago
|
||
Assignee | ||
Comment 19•23 years ago
|
||
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
Updated•23 years ago
|
OS: Windows NT → All
Assignee | ||
Comment 20•23 years ago
|
||
Assignee | ||
Comment 21•23 years ago
|
||
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.
Comment 22•23 years ago
|
||
In your latest patch, I don't see any changes to a jar.mn file. Should there be?
Assignee | ||
Comment 23•23 years ago
|
||
Yes! Same change as before -- I wonder how that happened!
Assignee | ||
Comment 24•23 years ago
|
||
Comment 25•23 years ago
|
||
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)
Assignee | ||
Comment 26•23 years ago
|
||
Assignee | ||
Comment 27•23 years ago
|
||
Assignee | ||
Comment 28•23 years ago
|
||
"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.
Assignee | ||
Updated•23 years ago
|
Whiteboard: [dialog]advanced edit → [dialog]advanced edit FIX IN HAND need r=, sr=
Assignee | ||
Comment 29•23 years ago
|
||
Assignee | ||
Comment 30•23 years ago
|
||
Assignee | ||
Comment 31•23 years ago
|
||
Assignee | ||
Comment 32•23 years ago
|
||
Assignee | ||
Comment 33•23 years ago
|
||
Comment 34•23 years ago
|
||
* 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...
Assignee | ||
Comment 35•23 years ago
|
||
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.
Comment 36•23 years ago
|
||
Rather than having "bold" (or italic) items in the menu, how about we have a
button to "prefill all required attributes"?
Assignee | ||
Comment 37•23 years ago
|
||
Assignee | ||
Comment 38•23 years ago
|
||
Assignee | ||
Comment 39•23 years ago
|
||
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.
Comment 40•23 years ago
|
||
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)
Assignee | ||
Comment 41•23 years ago
|
||
Assignee | ||
Comment 42•23 years ago
|
||
Assignee | ||
Comment 43•23 years ago
|
||
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.
Assignee | ||
Comment 44•23 years ago
|
||
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
Assignee | ||
Comment 45•23 years ago
|
||
Assignee | ||
Comment 46•23 years ago
|
||
Assignee | ||
Comment 47•23 years ago
|
||
Assignee | ||
Comment 48•23 years ago
|
||
Assignee | ||
Comment 49•23 years ago
|
||
Comment 50•23 years ago
|
||
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=
Comment 51•23 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Updated•23 years ago
|
Whiteboard: [dialog]advanced edit FIX IN HAND need a= → [dialog][advanced edit]fixed, reviewed, a=asa
Assignee | ||
Comment 52•23 years ago
|
||
checked in. Yea!
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 53•23 years ago
|
||
Charley is gonna help QA by verifying this one and marking VERIFIED-FIXED.
thanks Charley!
Assignee | ||
Comment 54•23 years ago
|
||
Sujay: Not a big deal, but you can verify it, can't you? It works!
Status: RESOLVED → VERIFIED
Comment 55•23 years ago
|
||
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.
Description
•