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

RESOLVED FIXED in mozilla15

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: mounir)

Tracking

(Blocks: 1 bug, {access})

Trunk
mozilla15
access
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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>
(Assignee)

Updated

5 years ago
Version: unspecified → Trunk
Mounir, should :disabled match such options?
(Assignee)

Comment 2

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

Yes, definitely.
OK, sounds like this should be pretty easy to fix, then...
(Assignee)

Comment 4

5 years ago
They should not have their disable state inherited from <select> AFAICT.
Summary: HTML option doesn't have disabled state inherited from disabled optgroup or select → HTML option doesn't have disabled state inherited from disabled optgroup
(Assignee)

Comment 5

5 years ago
Created attachment 628838 [details] [diff] [review]
Patch
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #628838 - Flags: review?(bzbarsky)
(Assignee)

Updated

5 years ago
Blocks: 577306
Comment on attachment 628838 [details] [diff] [review]
Patch

r=me
Attachment #628838 - Flags: review?(bzbarsky) → review+
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').
(Reporter)

Comment 8

5 years ago
(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
(Assignee)

Comment 9

5 years ago
(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?
(Reporter)

Comment 10

5 years ago
(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.
(Assignee)

Updated

5 years ago
Flags: in-testsuite+
Target Milestone: --- → mozilla15
(Assignee)

Updated

5 years ago
Attachment #628838 - Flags: checkin+
(Assignee)

Comment 11

5 years ago
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?
(Reporter)

Comment 12

5 years ago
Nevermind, if HTML shouldn't do that then we handle that on a11y side.
> 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.
https://hg.mozilla.org/mozilla-central/rev/654fb530a968
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.