14836 - [BETA] Disabled controls should not respond to mouse clicks

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Michael Lowe]

Upon opening the Editor "Insert Image" dialog, some of the button and radio
button control at the bottom of the dialog are disabled.   Even though these
controls are disabled they still respond to mouse clicks as if they were
enabled.   This should not be the case.

Comment 1

[karnaze (gone)]

Reassigning to Kevin.

Comment 2

[Kevin McCluskey (gone)]

mozilla/editor/ui/dialogs/content/EdImageProps.xul EdImageProps.js

Using the debugger, I don't see the radio button or other controls being
disabled. I think all that is happening is that appearance is being changed by
setting a class attribute to disabled or enabled and then having a style rule to
change how it is rendered. I don't see it applying a disabled attribute to the
form elements.

Changing component to editor and reassigning.

Comment 3

[Kathleen :Brade]

reassign to myself for all platforms.

Comment 4

[Michael Lowe]

This is also a problem in the "Customise Sidebar" dialog, and maybe others.

Comment 5

[Matthew Tuck [:CodeMachine]]

Dupe of bug #15112?

Comment 6

[Alec Flett]

Possibly.
I have (what I believe is) the fix for this in my tree... just need to get it
reviewed by Joki..

Comment 7

[rubydoo123]

Charley revamped the Image Dialog box and the buttons and controls are now
enabled and functional. Using the 19991102 build, the buttons and
controls work fine. Marking this bug fixed.

Comment 8

[Michael Lowe]

But do are disabled controls behaving as is they were enabled?   Having the
enabled controls working now is not sufficient to close this bug.

Comment 9

[rubydoo123]

yes, that is why I marked the bug fixed, the appropriate behavior is there for
enabled and disabled controls -- i.e. selecting "ok" when it is disabled
produces an error dialog.

Comment 10

[Michael Lowe]

Ah, but the appropriate behavior when you click on a disabled ok control is for
the control not to fire back an event to the application (so you would not get
any error message back at all).

Comment 11

[rubydoo123]

yes you should, if you don't select an image and then select ok, you should be
prompted (told) that you need to select an image. It would be inappropriate for
no user feedback

Comment 12

[Michael Lowe]

A disabled control is a _disabled_ control.  On Windows at least, hiting a
disabled control does not provide any feedback.   I'm fairly sure other
operating system are the same.  The user knows not to hit a disabled control
because it is dimmed.

Comment 13

[rubydoo123]

I installed todays build and now it is broke. The behavior is totally different
than yesterdays build. So, I will reopen and reassign to cmanske, he checked in
changes last night which altered the dialog.

Comment 14

[Charles Manske]

All of the input, select, and titledbuttons are getting the "disabled"
attribute set correctly. This is not an editor problem.
I can change the logic so that the editor does
the right thing, but the basic problem is that the radio buttons and
titledbuttons fire the "onclick" handler even when they are disabled.
They shouldn't do that, correct?
The text (input type="text") and select widgets seem to work Ok.

Comment 15

[Kevin McCluskey (gone)]

Ok, I just pulled Nov 4, 1999 12:58PM and it looks like the image property
dialog is working correctly as far as it is defined.

There are two things going on.  diabled + styling to look deactivated. The "ok"
button is styled to look deactivated but is NOT disabled(i.e it does not have
the disable attribute set.) Instead it sets it's class attribute to be
"disabled". The class attribute is used to pick up the "deactivated" style for
the buttons. Setting the class attribute to "disable" does not disable the
element. It looks like this is intentional. When I click on the "ok" button that
has been styled to look deactivated it brings up an alert dialog informing me to
fill in some more fields. If the button where disabled, clicking on the button
would not generate a click event - so the alert dialog would never be displayed.

In addition,  I made sure that radio buttons honor the disable attribute by
inserting a new <input type="radio" disabled="true"> tag into the
EdImageProps.xul. It shows up as deactivated radio button that does NOT respond
to clicks.

I'm marking this bug as WORKSFORME. Please reopen if I'm missing the point, but
as far as I can tell the radio button is doing exactly what the XUL and js are
specifying.
Looking at the

Comment 16

[Michael Lowe]

Based upon comments by [e-mail], this seems like a bug in the
editor javascript code as implemented.

Comment 17

[Charles Manske]

I just repulled EdImageProps.js and EdImageProps.xul.
You aren't reading the JS correctly or you aren't getting the latest version.
Search through EdImageProps.js and you'll see that we use SetElementEnabledByID
to set the disabled attribute for all widgets. This method is in
EdDialogCommon.js:
function SetElementEnabledByID( elementID, doEnable )
{
  element = document.getElementById(elementID);
  if ( element )
  {
    if ( doEnable )
    {
      element.removeAttribute( "disabled" );
    }
    else
    {
      element.setAttribute( "disabled", "true" );
    }
  }
}
Isn't this correct?
Yes, there is also a method called SetClassEnabledByID, which sets a class
attribute to "disabled", but we are no longer using that. It is not being
called at all.
The radio buttons are set disabled="true" by JS when you first launch to dialog.
Click on either radio button and the onclick handler (doDimensionEnabling)
is called -- it shouldn't be.
Could there be a difference between including 'disabled="true"' in XUL and
setting it via JS setAttribute()?

Comment 18

[Kevin McCluskey (gone)]

Oops, I was looking at the old file which did call SetClassEnabledByID.
The following simple test case fails - It calls the onclick handler even though
it is disabled:

<html>
 <body>
  <form>
    <input disabled="true" type="button" value="button"
onclick="dump('button\n');">
    <input disabled="true" type="radio" name="r" value="radio1"
onclick="dump('radio1\n')">
    <input disabled="true" type="radio" name="r" value="radio2"
onclick="dump('radio2\n')">
    <input disabled="true" type="radio" name="r" value="radio3"
onclick="dump('radio3\n')">
  </form>
 </body>
</html>

The problem is we listen for the MouseClick in the event handler for the
Frame. The MouseClick has already been processed by DOM and JS before the
frame's MouseClick is called.

I think we need to do one of the following:

Either we need to listen to the MouseDown and consume the event if the button is
disabled or we need to listen for the mouseclick by making the button's DOM
event listeners. If the button is disabled we need to consume the event.

Comment 19

[Kevin McCluskey (gone)]

Tom, here is the bug we discussed. When you have disabled working within the
EventState manager let me know so I can remove the code in the form elements
which looks at the disabled attribute.

Comment 20

[rickg]

Moving to m13 because Joki seems to be distracted.

Comment 21

[gerardok]

QA Contact update.

Comment 22

[joki (gone)]

Okay, fix checked in.  Handlers on disabled elements will no longer fire.

Kevin, if the code you're talking about removing is in frame event handling I
suggest you leave it in.  In doing the fix it became apparent we don't really
have a good central location for this so I fixed it at the DOM event dispatch
level which means frames will still get events and need to check disabled state
themselves but I don't think that's really a big deal.

Comment 23

[Phil Peterson]

*** Bug 15112 has been marked as a duplicate of this bug. ***

Comment 24

[ckritzer (gone)]

Marking VERIFIED FIXED on:
- Linux6 2000-02-01-10 Commercial build
- Win98 2000-02-01-08 Commercial build
- MacOS86 2000-02-01-09 Commercial build