New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removeAttr("selected") should not set property to false #1759

Closed
mgol opened this Issue Oct 21, 2014 · 13 comments

Comments

Projects
None yet
5 participants
@mgol
Member

mgol commented Oct 21, 2014

Originally reported by wchen@… at: http://bugs.jquery.com/ticket/14633

 http://jsfiddle.net/b85D4/5/

In Firefox the selected value is 1. In Chrome the selected value is 2.

removeAttr has special handing for boolean attributes to set the corresponding DOM property to false:  https://github.com/jquery/jquery/blob/master/src/attributes/attr.js#L86

Removing an attribute and setting the DOM property to false are very different. In this case, as specified ( http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#dom-option-selected), setting the property to false means setting the dirtiness to true. This changes how the option should react to "selected" attribute changes. In the test case, it means that adding the "selected" attribute at the end should not select the option.

Firefox respects the specified behaviour and does not select the second option.

Issue reported for jQuery 2.0.3

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 21, 2014

Member

Comment author: dmethvin

I agree this is the right thing to do, but perhaps not the compatible thing. Let's kick this can down the road for now.

Member

mgol commented Oct 21, 2014

Comment author: dmethvin

I agree this is the right thing to do, but perhaps not the compatible thing. Let's kick this can down the road for now.

@AurelioDeRosa

This comment has been minimized.

Show comment
Hide comment
@AurelioDeRosa

AurelioDeRosa Apr 3, 2015

Member

Will this change be landed in 3.0?

Member

AurelioDeRosa commented Apr 3, 2015

Will this change be landed in 3.0?

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jan 13, 2016

@timmywil Is there an ETA on when this will actually appear in a jQuery release? Right now doing this:

            $('#foo').removeAttr('selected');
            $('#foo').attr('selected', 'selected');

on an <option id="foo"> will leave it unselected. And people keep reporting this as a browser bug every so often...

bzbarsky commented Jan 13, 2016

@timmywil Is there an ETA on when this will actually appear in a jQuery release? Right now doing this:

            $('#foo').removeAttr('selected');
            $('#foo').attr('selected', 'selected');

on an <option id="foo"> will leave it unselected. And people keep reporting this as a browser bug every so often...

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 13, 2016

Member

@bzbarsky That is a different issue. To set the dynamic selected state, use the property instead of the attribute. The selected attribute is used to set the default state for when the page loads.

$("#foo").prop("selected", true);
Member

timmywil commented Jan 13, 2016

@bzbarsky That is a different issue. To set the dynamic selected state, use the property instead of the attribute. The selected attribute is used to set the default state for when the page loads.

$("#foo").prop("selected", true);
@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jan 13, 2016

Member

I don't see a difference in behavior though: http://jsbin.com/lipunucamu/edit?html,js,output

Member

dmethvin commented Jan 13, 2016

I don't see a difference in behavior though: http://jsbin.com/lipunucamu/edit?html,js,output

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 13, 2016

Member

@dmethvin You can set the default state like that if the element hasn't been interacted with yet.

Member

timmywil commented Jan 13, 2016

@dmethvin You can set the default state like that if the element hasn't been interacted with yet.

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jan 13, 2016

@timmywil The issue with the code I quoted is that removeAttr sets both the default state and the dynamic state. But attr only sets the default state. So the option ends up not selected. The fact that removeAttr sets the dynamic state is precisly what this issue is about, no?

bzbarsky commented Jan 13, 2016

@timmywil The issue with the code I quoted is that removeAttr sets both the default state and the dynamic state. But attr only sets the default state. So the option ends up not selected. The fact that removeAttr sets the dynamic state is precisly what this issue is about, no?

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 13, 2016

Member

@bzbarsky It's expected behavior. Browsers may differ on when they consider the element to no longer be "fresh". I haven't checked them all. When the element is still fresh, setting the default state will set the dynamic state as well. But I can tell you that the proper way to set the dynamic state is with the property.

Member

timmywil commented Jan 13, 2016

@bzbarsky It's expected behavior. Browsers may differ on when they consider the element to no longer be "fresh". I haven't checked them all. When the element is still fresh, setting the default state will set the dynamic state as well. But I can tell you that the proper way to set the dynamic state is with the property.

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jan 13, 2016

You can tell me, but I already know this. From my point of view the proper way to set this state is without even involving jQuery at all. The problem are the people who keep running into this inconsistency between attr and removeAttr and then filing bugs on browsers.

But I'm not even sure what we're arguing about here. The fix for this issue makes the change I think should be made; I'm just asking when it's planned to ship.

bzbarsky commented Jan 13, 2016

You can tell me, but I already know this. From my point of view the proper way to set this state is without even involving jQuery at all. The problem are the people who keep running into this inconsistency between attr and removeAttr and then filing bugs on browsers.

But I'm not even sure what we're arguing about here. The fix for this issue makes the change I think should be made; I'm just asking when it's planned to ship.

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 13, 2016

Member

It will be shipped with the 3.0 release.

Member

timmywil commented Jan 13, 2016

It will be shipped with the 3.0 release.

@AurelioDeRosa

This comment has been minimized.

Show comment
Hide comment
@AurelioDeRosa

AurelioDeRosa Apr 19, 2016

Member

Should I open a ticket in the api repo to document this?

Member

AurelioDeRosa commented Apr 19, 2016

Should I open a ticket in the api repo to document this?

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Apr 19, 2016

Member

@AurelioDeRosa Yes, I think so. Most tickets with the Behavior change label need docs changes.

Member

mgol commented Apr 19, 2016

@AurelioDeRosa Yes, I think so. Most tickets with the Behavior change label need docs changes.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Apr 19, 2016

Member

We should be able to take a lot of wording and generate issues from the upgrade guide which is currently a Google Doc.

Member

dmethvin commented Apr 19, 2016

We should be able to take a lot of wording and generate issues from the upgrade guide which is currently a Google Doc.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.