Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Checkboxradio: Fixed a bug that the input type="radio" button without the name attribute is not changed to the checked status when it is clicked. #6659

Conversation

hyunsook
Copy link
Contributor

Hi All~!

If a page does not include jQM but include jQuery, the input type="radio" button is changed the checked status properly when it is clicked although the radio button doesn't have the name attribute.
But, if jQM is added on that page, the radio button is not changed the checked status.

To fix this bug, I submit this PR.

If there are any problems or mistakes on that PR, please let me know.
Thanks in advance. :)

@hyunsook
Copy link
Contributor Author

For your information, the input type="checkbox" button without the name attribute is changed to the checked status properly for both cases, due to the code block of #L187. : https://github.com/jquery/jquery-mobile/blob/master/js/widgets/forms/checkboxradio.js#L187

@gabrielschulhof
Copy link

Good catch!

@gabrielschulhof
Copy link

This is not a regression from 1.3.2: http://jsbin.com/ofuhaw/711/

@hyunsook
Copy link
Contributor Author

@gabrielschulhof : Thanks. :)

Yes, 1.3.2 also has this issue/bug.
My PR doesn't mean that 1.4.0 is a regression from 1.3.2.
I think that a radio button in a page with jQM, 1.4.0 or 1.3.2, should be operated exactly like a radio button in a page without jQM. To do so, I submitted that PR.

@arschmitz
Copy link
Contributor

@hyunsook you are correct there is a bug that needs to be fixed the reason @gabrielschulhof mentions it is not a regression is because at the point we are in the release process unless something is a major regression any fixes will need to wait for 1.4.1 unless its substantial enough to warrant a second RC

… the name attribute is not changed to the checked status when it is clicked.
@arschmitz
Copy link
Contributor

@hyunsook Can you please update your git config to use your full name matching what you signed the cla with. After that i think this is good to go.

@gabrielschulhof gabrielschulhof self-assigned this Feb 7, 2014
@gabrielschulhof
Copy link

I've fixed those things and I've moved the commit to #7082.

@gabrielschulhof
Copy link

I guess this PR should auto-close when I apply #7082.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a6e4bb5 on hyunsook:pr-1.4-checkoutradio-radio-getInputSet into * on jquery:master*.

@gabrielschulhof
Copy link

In the end, we've decided to go with a simpler solution: Simply add this.element to the return value of _getInputSet().

gabrielschulhof pushed a commit that referenced this pull request Feb 9, 2014
@gabrielschulhof gabrielschulhof modified the milestones: 1.4.2, 1.4.1 Feb 13, 2014
gabrielschulhof pushed a commit that referenced this pull request Feb 17, 2014
gabrielschulhof pushed a commit that referenced this pull request Feb 20, 2014
gabrielschulhof pushed a commit that referenced this pull request Feb 23, 2014
@hyunsook
Copy link
Contributor Author

@arschmitz : Oh no! I'm truly sorry I wasn't aware of your message.
@gabrielschulhof : My PR apparently wasn't merged because the solution for that issue was changed. right? It's a pity...... But, there's always next time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants