759666 - HTML option doesn't have disabled state inherited from disabled optgroup

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[alexander :surkov (:asurkov)]

HTMLOptionElement->State().HasState(NS_EVENT_STATE_DISABLED) is false:

<select size="3" disabled>
  <option>option</option>
</select>

<select size="3">
  <optgroup disabled>
    <option>option</option>
  </optgroup>
</select>

Comment 1

[Boris Zbarsky [:bzbarsky]]

Mounir, should :disabled match such options?

Comment 2

[Mounir Lamouri (:mounir)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> Mounir, should :disabled match such options?

Yes, definitely.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK, sounds like this should be pretty easy to fix, then...

Comment 4

[Mounir Lamouri (:mounir)]

They should not have their disable state inherited from <select> AFAICT.

Comment 5

[Mounir Lamouri (:mounir)]

Attachment: Patch

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 628838 [details] [diff] [review]
Patch

r=me

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 628838 [details] [diff] [review]
Patch

Er, I forgot the review comments:

>+++ b/content/html/content/src/nsHTMLOptGroupElement.cpp
>+nsHTMLOptGroupElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
...
>+    // All our children <option> have there :disabled depending on our

s/there/their/ and add "state" after ":disabled".

>+    // disabled attribute. We should make sure there state is updated.

s/there/their/

>+        // No need to call |IsELement()| because it's an HTML element.

"IsElement" (lowercase 'l').

Comment 8

[alexander :surkov (:asurkov)]

(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #4)
> They should not have their disable state inherited from <select> AFAICT.

why? they are disabled as well and no difference from user perspective

Comment 9

[Mounir Lamouri (:mounir)]

(In reply to alexander :surkov from comment #8)
> (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #4)
> > They should not have their disable state inherited from <select> AFAICT.
> 
> why? they are disabled as well and no difference from user perspective

They are not really disabled, the select is disabled. In a user perspective, that would make no difference because a disabled <select> shouldn't show its options. Is that annoying for some a11y-specific reason?

Comment 10

[alexander :surkov (:asurkov)]

(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #9)
> (In reply to alexander :surkov from comment #8)
> > (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #4)
> > > They should not have their disable state inherited from <select> AFAICT.
> > 
> > why? they are disabled as well and no difference from user perspective
> 
> They are not really disabled, the select is disabled. In a user perspective,
> that would make no difference because a disabled <select> shouldn't show its
> options. 

> Is that annoying for some a11y-specific reason?

yes. what a11y reports is close to user's perception of UI. If option is not available (it doesn't matter whether its parent control is disabled or the option itself is disabled) then we report that option is not available.

From technical perspective all we do now is checking the disabled state on element assuming it's inherited. If it's not then we should do something like:

if element has not disabled state and if that's a select option then get its parent select element and check its disabled state.

actually we do that for XUL (where iirc element states don't work due to some reason) but I'd be happy to get that for free.

Comment 11

[Mounir Lamouri (:mounir)]

Unfortunately, I don't think there is any reason - that are going to be accepted - to push that to the HTML specifications. I would tend to say that special casing is likely the good solution here. You can make this very simple with the ::GetSelect() method.
Boris, do you see any other solution?

Comment 12

[alexander :surkov (:asurkov)]

Nevermind, if HTML shouldn't do that then we handle that on a11y side.

Comment 13

[Boris Zbarsky [:bzbarsky]]

> Boris, do you see any other solution?

Not offhand.

Note that a disabled <select multiple> does in fact show the options, so the difference is perceivable via styles on the options... but I agree that from an HTML point of view those options should not be treated as disabled.

Comment 14

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/654fb530a968