Checkboxradio: Retrieve label correctly#7293
Conversation
|
Hmmm ... there seems to be a perf hit especially on Blink: http://jsperf.com/checkboxradio-label-retrieval-non-nested |
Improves performance a great deal when the .labels property is supported, and also improves performance when such a property is not supported (FF27). http://jsperf.com/checkboxradio-label-retrieval-non-nested/5 Closes gh-7293 Fixes gh-7292
This helps render the code unit-testable.
Improves performance a great deal when the .labels property is supported, and also improves performance when such a property is not supported (FF27). http://jsperf.com/checkboxradio-label-retrieval-non-nested/5 Closes gh-7293 Fixes gh-7292
|
OK. Perf issues fixed thanks to @arschmitz, yielding an enormous improvement on platforms that support the |
|
Is this fix only relevant for inputs or are there other form elements that could gain in performance when locating the relevant label? |
|
anyone? |
|
@Ruffio I believe it's only relevant to checkboxes. |
There was a problem hiding this comment.
This sounds like a bug in core have you checked with them about this?
There was a problem hiding this comment.
That wasn't my finding. I moved the comment along with the rest of the code.
|
@dmethvin what do you think about a selector not finding an element but filter doing it in WP8? If this is true sounds like a core bug to me. |
|
Is there a test case? |
|
@dmethvin @arschmitz I'll try to see if this is reproducible and if so, I'll try to build up a test case for core. |
|
Thanks, it sounds like it might be an IE11 issue in general, so the test case will help there and maybe we can get them to fix it. |
There was a problem hiding this comment.
@arschmitz Since we're going to be using getElementsByTagName(), we're keeping the .filter() setup, but not because otherwise it doesn't work on Windows.
There was a problem hiding this comment.
Why are you searching this.document[0] with getElementsByTagName is this a perf optimization?
|
👍 |
This moves label-finding code into one function to help render the code unit-testable. The new code also relies more on native means of getting the label, which improves performance a great deal when the .labels property is supported, and also improves performance when such a property is not supported (such as in FF27). http://jsperf.com/checkboxradio-label-retrieval-non-nested/5 (cherry picked from commit d9f5d21) Closes gh-7293 Fixes gh-7292
This moves label-finding code into one function to help render the code unit-testable. The new code also relies more on native means of getting the label, which improves performance a great deal when the .labels property is supported, and also improves performance when such a property is not supported (such as in FF27). http://jsperf.com/checkboxradio-label-retrieval-non-nested/5 Closes jquery-archivegh-7293 Fixes jquery-archivegh-7292
5 participants
This PR does two things. It
_findLabel) so it's easily unit-testableI've run the new unit tests on:
... and all the right ones fail without the change applied, and all of them pass with the change applied.