306162 - disabled="false" disables an editable <menulist> or <textbox> partly

Status: NEW

(Core :: XUL, defect)

Description

[Mats Palmgren (inactive)]

Somewhat related to bug 305840 since nsXULElement::IsFocusable() uses it:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xul/content/src/nsXULElement.cpp&rev=1.579&root=/cvsroot&mark=692-695#685

STEPS TO REPRODUCE
1. Add the flollowing block of code at a suitable place, for example in
the XUL part of IsFocusable() in nsEventStateManager.cpp:

printf("XUL focusFrame=%p aContent=%p\n", focusFrame, aContent);
if (1) {
  printf(" ++++++++++++++++++++++++++++++++++++++++\n");
  nsIFrame* f = focusFrame->GetParent();
  nsIFrameDebug* fdbg;
  f->QueryInterface(NS_GET_IID(nsIFrameDebug), (void**) &fdbg);
  if (fdbg)
    fdbg->List(f->GetPresContext(), stdout, 0);
  printf(" ----------------------------------------\n");
}

if (1) {
  nsCOMPtr<nsIDOMXULControlElement> xulControl = do_QueryInterface(aContent);
  if (xulControl) {
    PRBool disabled = PR_FALSE;
    xulControl->GetDisabled(&disabled);
    printf("aContent implements nsIDOMXULControlElement, disabled=%d\n", disabled);
  }
}

You need to include these too:
#include <nsIDOMXULMenuListElement.h>
#include <nsIFrameDebug.h>


ACTUAL RESULTS
See attached trace file.

EXPECTED RESULTS
disabled=1

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 2

[Mats Palmgren (inactive)]

Attachment: Trace log

Comment 3

[Mats Palmgren (inactive)]

Oh, I see that disabled="disabled" is probably invalid, it should be
disabled="true". I still think there is a problem here since disabled="disabled"
does not affect the menulist but it does make the inner textbox disabled.

Comment 4

[[e-mail]]

Presumably <textbox disabled="disabled"> mostly disables the textbox (I'm not
sure it appears disabled but at least it thinks it's disabled).

Comment 5

[Mats Palmgren (inactive)]

I guess we just forward the disabled attribute to the inner html:input verbatim
and since disabled="disabled" is valid HTML it has an effect there but not
on the menulist. Maybe we shouldn't forward invalid attribute values?
Feel free to resolve as INVALID/WONTFIX.

Comment 6

[Mats Palmgren (inactive)]

Essentially the same occurs for <textbox> -- the outer textbox is not
disabled (and thus focusable) and vice versa for the inner html:input.
It's a bit confusing for a XUL novice like myself :-)

Comment 7

[[e-mail]]

So, we're agreed that <textbox disabled="true"/> and <menulist editable="true"
disabled="true"/> work as expected? We can probably make <textbox
disabled="disabled"> do the right thing, but <menulist editable="true"
disabled="disabled"> won't work without some native theme code fixes.

Comment 8

[Mats Palmgren (inactive)]

(In reply to comment #7)
> So, we're agreed that <textbox disabled="true"/> and <menulist editable="true"
> disabled="true"/> work as expected?

Yes. But the bug also occurs for disabled="false" which I assume should work
the same as "not specified".

> We can probably make <textbox disabled="disabled"> do the right thing,

Which is?

According to
http://www.xulplanet.com/references/elemref/ref_textbox.html#attr_disabled

this attribute is of type "boolean" which I would assume means "true" or
"false" but the text then goes on to say:
"To enable the textbox, leave the attribute out entirely as opposed to setting
the value to false."

which I think is wrong. One should be able to specify disabled="false" IMHO.
(Is the text there a direct reflection of this bug?)

In HTML4 the value for the 'disabled' attribute is implied, so what
we need to do is to map <textbox disabled="true"> to disabled="disabled"
(or any value really) for the inner html:input and any other value (including
"disabled") to an unspecified value.

Comment 9

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 10

[[e-mail]]

I was thinking that we just had to ensure that the disabled property reflected
reality, then it occurred to me that we have to ensure that the CSS reflects
reality, then it occurred to me that we have to fix native theming too.

Disabling the HTML input/textarea only when the textbox's disabled attribute is
set to true is probably also a non-starter :-(

Comment 11

[Todd Whiteman]

This is insane, setting:
  <textbox disabled="false" />
in XUL will disable a textbox on Linux.

Anyone have an idea for coming up with a fix to this?