djoham Reporter
	Description • 24 years ago

From Bugzilla Helper:
User-Agent: Mozilla/4.7 [en] (X11; I; Linux 2.2.14-15mdk i686)
BuildID:    2000053008

If you have a button element on a form (and perhaps others, I haven't checked)
with a style associated with it, the style attributes will be lost if you change
the display property of the element

Reproducible: Always
Steps to Reproduce:
1.Open the test case. Notice that the button on the right has a style associated
with it. Click on the button on the right. Notice that the button on the left is
now disabled (expected) and has lost its style attributes (doh!)
2.
3.

Actual Results:  The style element of the button being disabled is lost

Expected Results:  I would expect the button to be disabled, but retain the
style elements (size, font, etc)	

IE renders as I would expect

	David Baron :dbaron:
	Comment 2 • 24 years ago

Confirming bug, Linux 2000-06-01-08-M16.

	rods (gone)
	Comment 4 • 24 years ago

Looks like a style issue

	Marc Attinasi
	Comment 5 • 24 years ago

Hey, this is a cool one. If the testcase is changed to not use class selectors 
then it works fine... I'll keep looking to understand why. Clearly it is a style 
problem though.

Can anybody give me a suggestion as to the impact of this bug?

	Marc Attinasi
	Comment 6 • 24 years ago

The test cases attached don't work for me... but I have one that does and the
bug appears to have been fixed, presumably bu shaver's recent fix to deal with
classes in selectors when determining if a an attribute affects style.

Rod, please veryify that this works for you too.

	djoham Reporter
	Comment 8 • 24 years ago

This must have regressed. Marc, I see that your test
case is still working, but mine is not. I think I have
figured out why tho....

Your test case uses the simple style of "button" for
all button elements. If I do the same thing in my test
case, the system works as expected. However,
I've added a Button.XXXXX style so that I can have
button elements with different styles. As soon as I
do this, Mozilla seems to get confused about which
style to apply to the button(s). I'll submit the test
cases, but this is what I've found so far:

test 1:
one button and one style called button.global-CancelButton
(This is the original test case)

result:
button looses style after being being disabled

------------------------------------------

test 2:
one button and one style called button
(Marc's test case)

result:
button retains style as expected

------------------------------------------

test 3:
two buttons and two styles (button.green and button.red)

result:
both buttons loose style after being disabled

------------------------------------------

test 4:
two buttons and two styles (button and button.red)

result:
*both* buttons revert to the button style after
being disabled. Mozilla is very confused!



What do you think the priority on this is? In my opinion, this is rather
important to people who are trying to build we applications with Mozilla.
Disabling and enabling buttons is rather common. I would personally like to
see this fixed by beta 3.

Thoughts?

David

	Marc Attinasi
	Comment 11 • 24 years ago

Hey David, I agree that this is pretty important, at least on the surface. I'll 
look into it further, and I'll nominate for beta3 as well. Approval or Denial 
for beta3 will come soon too.

	Marc Attinasi
	Comment 12 • 24 years ago

Works fine for <INPUT TYPE=BUTTON>. Also, with 
the environment NSPR_LOG_MODULES=styleverifytree:1 I see some style context 
parenting problems coming out of the <BUTTON> indicating that it is setting up 
it's style context incorrectly. My guess is that is the root of this problem.

	rods (gone)
	Comment 13 • 24 years ago

Here one of my commentsfrom Bug 49740:

"The real issue is that there is a built in assumption in the 
nsCSSFrameConstructor that all child will be added directly to the parent frame. 
This is wrong. There are several cases where the children are added to an 
intermediate frame and the problem is the children being added are having their 
new style context's resoved against the "assumed" parent frame instead of the 
"real" that they will soon be a child of. Marc and I discussed several was to 
fix this, but there is going to have to be a new call to either get the real 
parent frame or the "real" parent style context."

	Marc Attinasi
	Comment 14 • 24 years ago

Even with the parentage of the style contexts corrected this is still broken. 

I did notice that the two HTML buttons are sharing the same style context (that 
happens if two siblings have exactly the same rules) but since one of the 
buttons has additional rules (the .red rules) the sharing should not have taken 
place... That doesn'e explain why INPUT buttons are OK, but it gives me 
something else to investigate.

The change to the style context parentage needs to get checked in anyway: having 
the wrong parent style contexts can only mean trouble... I'll attach the diff as 
a patch.

	Marc Attinasi
	Comment 16 • 24 years ago

To clarify my last comment: the two button control frames share the same style 
context only after the 'press me' button is pressed and the buttons are 
disabled. Prior to disabling the buttons they have unique style contexts. Also, 
even if the buttons are re-enabled they continue to share the same style 
context. 

This is because the class on the 'red' button is lost after disabling it. 

Here is a content dump before disabling:

      button@0227DBA8 name=button id=button class=red refcount=5<
        Text@0227D880 refcount=3<Goober>
      >
      Text@0227D7D0 refcount=3<\n>
      button@0227D6D8 name=button2 id=button2 refcount=5<
        Text@0227D470 refcount=3<Goober>
      >
      Text@0227D3C0 refcount=3<\n\n>

and now after disabling:

      button@0227DBA8 name=button id=button class=DISABLED disabled= refcount=6<

        Text@0227D880 refcount=3<Goober>
      >
      Text@0227D7D0 refcount=3<\n>
      button@0227D6D8 name=button2 id=button2 disabled= class=DISABLED refcount=
6<
        Text@0227D470 refcount=3<Goober>
      >
      Text@0227D3C0 refcount=3<\n\n>

and now after re-enabling:

      Text@0227DCA0 refcount=3<\n>
      button@0227DBA8 name=button id=button class= refcount=6<
        Text@0227D880 refcount=3<Goober>
      >
      Text@0227D7D0 refcount=3<\n>
      button@0227D6D8 name=button2 id=button2 class= refcount=6<
        Text@0227D470 refcount=3<Goober>
      >
      Text@0227D3C0 refcount=3<\n\n>

Clearly, disabling the buttons is setting their class to DISABLED causing the 
original class to be lost, so from then on the buttons are suppossed to look 
identical, they have the same class (and the shared style context is correct).

So, why is the class set to DISABLED when the HTMLButton is disabled??? Any 
cluses, Rod? When we figure out this class=DISABLED problem the bug will be 
fixed.

	Marc Attinasi
	Comment 17 • 24 years ago

OK, here it is:

NS_IMETHODIMP                                                       
nsHTMLButtonElement::SetDisabled(PRBool aValue)                                
{                                                                   
  nsHTMLValue empty(eHTMLUnit_Empty);                              
  if (aValue) {                                                     
    nsresult status = mInner.SetHTMLAttribute(nsHTMLAtoms::disabled, empty, 
PR_TRUE); 
    mInner.SetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::kClass, 
NS_ConvertASCIItoUCS2("DISABLED"), PR_TRUE);
    return status;

  }                                                                 
  else {                                                            
    mInner.UnsetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::disabled, PR_TRUE);  
    mInner.SetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::kClass, nsAutoString(), 
PR_TRUE);
    return NS_OK;                                                   
  }                                                                 
}

See how it is setting both the disabled and class attributes? That is the 
problem. The class attribute cannot just be wiped out like that... 

Over to you, Rod, to fix or future. CC'ing myself to stay in the loop.

	rods (gone)
	Comment 18 • 24 years ago

If you have a form control that has a class set for style:

button.green
{
    COLOR: green;
    FONT-FAMILY: Arial, 'MS Sans Serif';
    FONT-SIZE: 12pt;
    FONT-WEIGHT: 700;
    WIDTH: 200px;
}
....
<button name="button" id="button" class="red">Goober</button><br>

If you set the element to be disabled via script:

document.frmForm.button.disabled = !document.frmForm.button2.disabled;

How do you get the element to display itself in its disabled style as define in 
the html.css?

If in nsHTMLButtonElement::SetDisabled I set the class attr to "DISABLED", it 
picks up the stle correctly:

mInner.SetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::kClass, 
NS_ConvertASCIItoUCS2("DISABLED"), 
PR_TRUE);

Put now the element has lost the original value of the "class" attr and when 
disabled gets set back to "false" the element all it can 
do is clear the class which is wrong. This seems to be a problem with not just 
the button element,but all the form controls, and 
maybe other elements in the DOM. The easiest solution would be to cache the 
original "class" value when it gets set to disanled 
and then make sure that if it is disabled and someone sets the class attr it 
sets  the cached value.

Is this the right solution?
Should it be implemented in a generic fashion for all elements?
How should it be implemented?

	Kevin McCluskey (gone)
	Comment 19 • 24 years ago

Marking nsbeta3+

	Hixie (not reading bugmail) Assignee
	Comment 20 • 24 years ago

rods: Why are you touching the "class" attribute at all? Just change the
"disabled" attribute.

	Marc Attinasi
	Comment 21 • 24 years ago

if html.css has a rule like:
  button[disabled] { color: gray; }

and the author's stylesheet has a rule like:
  button.green { color: green; }

a disabled button will show as green because the two rules have the same 
specificity (011) and the author rule cascades over the user agent rule. That 
explains why the rule for disabled buttons in html.css is not kicking in (unless 
the class on the button is removed, in which case it cannot match the 
button.green rule), and also why it worked fine when I put it in my author 
stylesheet (after the button.green rule of course).

Just wanted to clarify why the buttons seemed not be picking up the disabled 
style from html.css when the class was not getting changed, and seemed to be 
picking it up when the class was changed. I'm done now.

	rods (gone)
	Comment 22 • 24 years ago

fixed

	Hixie (not reading bugmail) Assignee
	Comment 23 • 24 years ago

Rod, your fix did not fix the problem. The comments from Marc Attinasi dated
2000-09-07 23:15 still apply and are still causing a problem.

Have a look at http://bugzilla.mozilla.org/showattachment.cgi?attach_id=9460
...for a good testcase. I will attach an even clearer one.

Why are we touching the 'class' attribute at all? What relevance does 'class'
have to whether something is disabled or not?

	Hixie (not reading bugmail) Assignee
	Comment 25 • 24 years ago

ChrisK: taking QA since I have a handle on this issue and you are swamped.
Hope that's ok dude. This bug is CSS-related anyway.

	ckritzer (gone)
	Comment 26 • 24 years ago

I have no ego in the matter.  If you wanna take my bugs, by all means! <grin>  
Thanks for the headsup, tho'...

	rods (gone)
	Comment 27 • 24 years ago

I'll take another quick look tomorrow, but this will prbably get minused

	rods (gone)
	Comment 28 • 24 years ago

Ok, you got me. What is wrong? It appears to be doing the correct thing right 
now. The btn's text color should be gray when it is disabled and green when 
enabled. This is what I am seeing.

	Kevin McCluskey (gone)
	Comment 29 • 24 years ago

Marking nsbeta3-.

	djoham Reporter
	Comment 30 • 24 years ago

For what it's worth, I agree with Rod. The buttons' text should be grey when 
it's disabled and whatever color CSS tells it to be when its enabled. As the 
reporter of this bug, I would consider it fixed if the decision was left up to 
me.

David

	Hixie (not reading bugmail) Assignee
	Comment 31 • 24 years ago

well, the problem is that at the moment there is no [standards compliant] way
to set the color of the button when it is disabled.

If you say

   button { color: green; }

...in your CSS file, then *according to the specs* you are setting it to green
regardless of the state of the [disabled] attribute.

If you want to only change it for the non-disabled state, you have to also set
the disabled state:

   button { color: green; }
   button[disabled] { color: gray }

So I would say this fix is invalid per the specs.

What is the argument for doing it our way rather than the spec's way?

(Note: If we want to change that back I can do it as part of my html.css changes
since I will be editing that part of the file anyway.)

	Hixie (not reading bugmail) Assignee
	Comment 32 • 24 years ago

We are already changing the border style anyway, BTW, so it's not like there 
will be no feedback that the button is disabled.

	Hixie (not reading bugmail) Assignee
	Comment 33 • 24 years ago

Taking bug since whatever is decided will go into my html.css (even if it is
"keep the current fix").

	rods (gone)
	Comment 34 • 24 years ago

Ok, I agree with your interpretation of the spec. Go for it.

	Hixie (not reading bugmail) Assignee
	Comment 35 • 24 years ago

Plussing, like all the other ua.css bugs I have fixed.

The 'fix' for this is basically to back out the previous fix, anyway, so look
at it this way: we are backing out the fix to this bug because it prooved to
be too high risk. ;-)

	clayton
	Comment 36 • 24 years ago

Ian, PDT has been avoiding comment on the us.css work.  But you need to know 
that with the imminent branch, the work is at risk, so your P1 marked bugs are 
getting PDT comments.

	djoham Reporter
	Comment 37 • 24 years ago

Ian, I'm a little confused about what fix you're going to (or thinking of)
backing out. This bug has kinda morphed from the original problem of loosing
styles to a discussion of what should happen when one particular form element is
disabled. I'm aware of a fix for the former (please don't back that one out!)
but not one for the latter. In retrospect, this should probably be a new bug at
this point.

Upon further reflection, I retract my original statement about having the button
text be gray when at all times when it's disabled. While I think this is a good
*default* solution, I should be able to override it if the mood strikes me with
the proper use of CSS.

Currently this is not possible ( button[disabled] {color : pink;} will not be
honored) in mozilla due to the following lines in html.css:

button[disabled]:-moz-text {
  color: gray;
  font:button;
}

I actually don't understand why that code is in html.css at all. This forces me
to do a non-standard CSS call (button[disabled]:-moz-text {color: pink;} ) in
order to change the font and color of disabled buttons. Why aren't these
attributes simply put in button[disabled]? As far as I can tell, button is the
only HTML attribute that forces me to do this. If I use a input[disabled] style,
the font color element is respected.

In my tree, I've moved the button[disabled]:-moz-text attributes up to
button[disabled] and removed that section of code entirely. I haven't seen any
regressions at all. Is there something I'm missing?

It would seem to me that the most appropriate solution (barring regressions) is
to remove the mozilla-specific CSS code and allow the web designer to do
whatever they like, assuming they use the proper CSS.

Thoughts?

David

	Hixie (not reading bugmail) Assignee
	Comment 38 • 24 years ago

That's exactly what I intend to do. :-)

	Blake Ross
	Comment 39 • 24 years ago

fixed, right?

	Hixie (not reading bugmail) Assignee
	Comment 40 • 24 years ago

FIXED by changes to html.css. 

Reporter: Please verify that the changes are ok. Thanks!

	Christine Hoffman
	Comment 41 • 24 years ago

Tested on Win, Mac and Linux. Verifying bug fixed. Style is retained as button 
is toggled.