Fix disabled option not updating when property changes #4818
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Consider the element's current disabled state instead of the cached one for disabled elements. This fixes the problem where disabled options couldn't be re-enabled.
Altered the line break to the correct format.
|
I never used this kind of workflow before, my bad on the line break. I tried creating a test for this specific issue but I still know very little of QUnit unfortunately. |
1 task
3 tasks
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
kevin-brown
added a commit
that referenced
this pull request
Jul 10, 2019
This check is in place in most other places, mostly because we have run into widespread issues under similar circumstances and we like to avoid those, but it was forgotten here. There also were no tests covering this, so it was never caught. This adds tests that ensure that the option in the results list will be generated with the correct "disabled" state based on whether or not it, or a parent element, is marked as disabled. This should have been easy: just check `element.disabled` Unfortunately the `disabled` property is not inherited within the option chain, so if an `<optgroup>` is disabled, the `<option>` elements or other `<optgroup>` elements held within it do not have their `disabled` property set to `true`. As a result, we needed to use the `matches` method to check if the `:disabled` state is present for the element. The `matches` method is part of the official standard, but it was not implemented under that name for a while and as a result Internet Explorer only supports it under the prefixed `msMatchesSelector` method and older versions of Webkit have it implemented as `webkitMatchesSelector`. But once we use this method, it appears to consistently return the expected results. This `matches` method and prefixed predecessors are not supported in IE 8, but they are supported in IE 9 and any browsers newer than that. Instead of buulding a very hacky solution using `querySelectorAll` that was brittle, I have chosen to act like everyone else and pretend IE 8 no longer exists. Fixes #3347 Closes #4818
3 tasks
kevin-brown
added a commit
that referenced
this pull request
Jul 10, 2019
This check is in place in most other places, mostly because we have run into widespread issues under similar circumstances and we like to avoid those, but it was forgotten here. There also were no tests covering this, so it was never caught. This adds tests that ensure that the option in the results list will be generated with the correct "disabled" state based on whether or not it, or a parent element, is marked as disabled. This should have been easy: just check `element.disabled` Unfortunately the `disabled` property is not inherited within the option chain, so if an `<optgroup>` is disabled, the `<option>` elements or other `<optgroup>` elements held within it do not have their `disabled` property set to `true`. As a result, we needed to use the `matches` method to check if the `:disabled` state is present for the element. The `matches` method is part of the official standard, but it was not implemented under that name for a while and as a result Internet Explorer only supports it under the prefixed `msMatchesSelector` method and older versions of Webkit have it implemented as `webkitMatchesSelector`. But once we use this method, it appears to consistently return the expected results. This `matches` method and prefixed predecessors are not supported in IE 8, but they are supported in IE 9 and any browsers newer than that. Instead of buulding a very hacky solution using `querySelectorAll` that was brittle, I have chosen to act like everyone else and pretend IE 8 no longer exists. Fixes #3347 Closes #4818
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the problem where disabled options couldn't be re-enabled.
This pull request includes a
The following changes were made
Fix for #3347