Disabled menulist or menulist with a <menupopup> with no child menuitems should not display a menu.

VERIFIED FIXED in mozilla0.9.5

Status

()

Core
XUL
--
trivial
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: TucsonTester2, Assigned: Charles Manske)

Tracking

Trunk
mozilla0.9.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

540 bytes, patch
Charles Manske
: review+
kinmoz
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

17 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.3+) Gecko/20010821
Netscape6/6.1b1
BuildID:    20010821

If you go into the characters and symbol in the insert menu and click on one of
the accent upper or lowercase menus then choose the common symbols menu a  very
small cut off menu appears for the Letter button instead of it simply
highlighting and not showing anything.

Reproducible: Always
Steps to Reproduce:
1.Open Composer
2.Click on insert and choose Characters and Symbols
3.Click on the Accent Uppercase radio button
4.Click on the Common symbols radio button
5.Click on the letter button

Actual Results:  There was a small cut off menu appearing after performing these
steps.  When the window first opened it was defaulted to the common symbols  and
I clicked on the letter box next to the character box, by doing this the letter
box just darkened.  After switching to one of the accent options and then
returning to the common symbols and clicking on the letter box it showed the
small menu instead of the darkened button.

Expected Results:  The highlighted button should be consistent throughout the
other menus, if there are options in the letter box they should show and if
there are none there should be a highlighted button displayed.  Maybe the button
can be greyed out entirely, but switching from one option to the next should not
effect how  the button acts for other options.

Comment 1

17 years ago
-->cmanske
Assignee: brade → cmanske
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

17 years ago
Very minor issue. Here's the cause: Initially, the "Letter" menulist doesn't 
have a child <menupopup>, so clicking on the disabled menulist shows nothing. 
But then we build the menulist when "Accented Upper" radio is used. After 
switching back to "Common Symbols", we clear the Letter menulist by removing all 
<menuitem>s, but not the <menupopup>. So it appears the in that state, clicking 
on the disabled menulist shows the very small empty box under it because of the 
<menupopup>.
This is really an XPFE issue. First, it seems that the <menupopup> should not 
attempt to draw itself at all if the menulist is disabled. But even if it does, 
it shouldn't show anything if there's no child menuitems.
Assignee: cmanske → hyatt
Component: Editor → XP Toolkit/Widgets: Menus
QA Contact: sujay → jrgm
Summary: Inserting characters and symbols menu graphic problem after choosing accent option and then switch back to common → Disabled menulist or menulist with a <menupopup> with no child menuitems should not display a menu.
Target Milestone: --- → mozilla1.0

Comment 3

17 years ago
Er, but that menulist isn't disabled. Don't you just want to do this?

Index: ui/dialogs/content/EdInsertChars.js
===================================================================
RCS file: /cvsroot/mozilla/editor/ui/dialogs/content/EdInsertChars.js,v
retrieving revision 1.8
diff -u -r1.8 EdInsertChars.js
--- ui/dialogs/content/EdInsertChars.js 2001/08/30 05:25:58     1.8
+++ ui/dialogs/content/EdInsertChars.js 2001/09/11 05:04:17
@@ -198,6 +198,7 @@
 function DisableLatinL(disable)
 {
   LatinL_Label.setAttribute("disabled", disable ? "true" : "false");
+  LatinL.setAttribute("disabled", disable ? "true" : "false");
 }

 function UpdateLatinL()



That fixes the defect in that dialog. (I'm going to punt for now on the 
issue about whether a menulist should show the popup if the popup has no
kids; I think there is already a bug on that though). 

-> cmanske, for checkin if that fix looks good to him

And, I'd just like to say to hewitt: Document Inspector rocks!! That makes
it really easy to go into some UI that you don't know and easily pick out 
what needs to be done. Thank you! Really nice work.
Assignee: hyatt → cmanske
Keywords: patch, review
QA Contact: jrgm → sujay
Target Milestone: mozilla1.0 → ---
(Assignee)

Comment 4

17 years ago
Doh! Sorry not to see that!
r=cmanske on the suggested fix.

Status: NEW → ASSIGNED
Whiteboard: FIX IN HAND need sr=
(Assignee)

Updated

17 years ago
Target Milestone: --- → mozilla0.9.5
(Assignee)

Comment 5

17 years ago
Created attachment 49151 [details] [diff] [review]
Fix.
(Assignee)

Updated

17 years ago
Attachment #49151 - Flags: review+
(Assignee)

Comment 6

17 years ago
Added John's fix as an attachment so we can do proper Status stuff there.

Comment 7

17 years ago
Did you mean to clear the cc: list?
(Assignee)

Comment 8

17 years ago
No, I didn't mean to remove you. Sorry!

Comment 9

17 years ago
Comment on attachment 49151 [details] [diff] [review]
Fix.

since LatinL_Label is no longer used we should be removing it from other places in that file.
Attachment #49151 - Flags: needs-work+

Comment 10

17 years ago
Comment on attachment 49151 [details] [diff] [review]
Fix.

sr=kin@netscape.com
Attachment #49151 - Flags: needs-work+ → superreview+

Updated

17 years ago
Whiteboard: FIX IN HAND need sr= → FIX IN HAND, reviewed

Comment 11

17 years ago
I don't understand the comment about 'LatinL_Label'. It is used. It's the 
text that (in English) says "Letter:", which should be disabled in conjunction
with its associated popup. [But anyways ... has sr/r, as is].
(Assignee)

Comment 12

17 years ago
I agree with John -- the label is still used.
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Keywords: patch, review
Resolution: --- → FIXED
Whiteboard: FIX IN HAND, reviewed

Comment 13

16 years ago
Verified
Status: RESOLVED → VERIFIED

Updated

10 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: sujay → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.